WebPositive SVG stroke rendering

[thebuck]

The handcrafted stroke-test.svg uncovers many stroke-related rendering issues, see Conclusions section.

Using the test image:

• View it standalone in WebPositive to test mouse hover effects and to avoid #17305.
  
• Every assymmetry (except color change on mouse hover) with respect to the vertical red line is a browser bug.
  

Elements of the image (left | right):

• orange top: filled rectangular path | stroked line path
  
• blue top U: just one curve path
  
• black O: differently scaled quarter paths with their stroke-width and geometry adjusted to make them look identical;
  listing stroke-width:
  
  • top: 4 | 0.125
    
  • bottom: 16 | 2.5
    
• blue bottom: line path segments | one dashed line path
  
• While the mouse is over an element, the element becomes green.
  

Conclusions from my hrev55788/x86_64/QEMU test run:

1. If 0 < stroke-width < 1 then it renders as 1.
2. transform is neglected for path polygonization detail level (same for fill).
3. stroke-dasharray is unimplemented.
4. ???
5. Renderer's bounding box of paths neglects stroke (however, extreme points of curves extend the box more than enough).
6. LCD subpixel antialiasing preference generally affects SVG (should always greyscale like other browsers).
7. The fill-rule equivalent for stroking is uninitialized (should be nonzero).
8. Renderer's bounding box of paths is one path-d unit too big to the bottom and right.

What tests led to conclusions:

1. The magenta zero-stroke thing is invisible, the O has thick top right (0.125 as 1), and bottom right was correctly not rounded to int (2.5).
2. The number of polylines (look carefully for corners) in O quarters increases with stroke-width, i.e. with less upscaling
3. The bottom blue right half is continuous, also for mouse hover detection (thin hover area because of 5.).
4. Scrolling the bounding box of the top left orange rectangle in/out of view toggles orange painting of all black elements (resize window for this). With LCD subpixel antialiasing only the fully covered pixels are aliased orange with a thin antialiasing rainbow border of black around. With greyscale antialiasing there is just a perfect orange stroke, but temporary black stroke glitches into view while scrolling. Only happens when encountering the combination "fill:none;stroke:#000" after an element with only fill but no stroke. Fixed in hrev55883.
5. Hovering the mouse over elements works only in their bounding box and repaints only the same (test especially blue elements; move between them and the red cross to get larger repaints).
6. Change antialiasing preference and reload page.
7. The red cross has a transparent center. I also saw that effect triggered by an unrelated transition somewhere else, without nearby "fill-rule:evenodd".
8. Hovering the top right orange line and considering point 5., the BBox is too big towards bottom right. The top right O quarter being orange as per point 4., then hovered, updates exactly one path-d unit (equals stroke-width with point 1.) too far to the bottom.

[thebuck]

version 3, to demonstrate also point 7.

[thebuck]

Most issues are actually in the app_server:

1. /haiku/src/servers/app/DrawState.cpp : DrawState::PenSize returns at least 1 (not in screen space). Even in screen pixel space this would make sense only with antialiasing disabled (no such option in haiku).
2. /haiku/src/servers/app/drawing/Painter/Painter.cpp : Painter::_StrokePath and _FillPath do not set approximation_scale on the VertexSource.
3. /haikuwebkit/Source/WebCore/platform/graphics/haiku/GraphicsContextHaiku.cpp : GraphicsContextHaiku::setLineDash requires app_server support first.
4. Likely in agg. How else could it know which pixels are fully covered?
5. /haikuwebkit/Source/WebCore/platform/graphics/haiku/PathHaiku.cpp : Path::strokeBoundingRect is used not only by the web inspector. Should be like /haikuwebkit/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp : CanvasRenderingContext2DBase::inflateStrokeRect.
6. app_server: Differentiate text and geometry antialiasing generally to prevent rainbow borders on geometry. Because text is usually small and pixel-hinted, artifacts are minimal there.
7. /haiku/src/servers/app/drawing/Painter/Painter.cpp : Painter::_StrokePath uses the fill rule that is set for Painter::_FillPath but should SetFillRule(B_NONZERO) temporarily.
8. /haikuwebkit/Source/WebCore/platform/graphics/haiku/FloatRectHaiku.cpp does ±1 a non-pixel size.

[thebuck]

Notes:

• app_server's Painter effectively ignores the bounds aka shapeFrame. This is good for now because Interface kit cannot DetermineBoundingBox() of a shape correctly because agg is not fully exposed.
• Do we really want to channel an aggressively introspective web browser through interface kit and app_server to reach agg? The same shape data is passed multiple times for filling, stroking, hit testing, and yet to come bounding box computation, length measurements... Why not let WebPositive use agg directly? Would increase security and even integration...

[pulkomandy]

Thanks for your time and efforts investigating and researching this.

Do we really want to channel an aggressively introspective web browser through interface kit and app_server to reach agg? The same shape data is passed multiple times for filling, stroking, hit testing, and yet to come bounding box computation, length measurements... Why not let WebPositive use agg directly? Would increase security and even integration...

If we were to do this, it would probably be not using agg, but cairo, which is already supported by WebKit. This would make a lot of things easier, since the cairo code is used by GTKWebKit and already well-tested there and in a few other places. However, it has the following disadvantages:

• The rendering will never look exactly the same as for native apps. By using the same API, we make sure WebKit renders exactly the same way as other applications, which is part of the efforts to provide an extremely consistent look and feel.
• Possibly usage of WebKit through remote_app_server would be faster by sending only drawing commands to the app_server, but that is probably only true to some extent (for simple webpages). Now the number of operations is so high that it probably is slower in the end than just sending a bitmap.
• It proves to be a very good stress test for app_server and interface kit APIs, and that seems important to me as an OS developer. If we are not able to deliver these nice complex drawing APIs to native apps, what is the point in writing all this OS? Surely everyone will write only web apps because they have more features there, and the native API just isn't good enough.

I agree that this approach may not be the best in the short term. We don't get a good working web browser because of it, and if we just used cairo (and curl for http) to power WebKit, we would have saved a lot of hours (or even days or months) of work for various people trying to get this to work.

It kind of worked, the native HTTP API is quite good (but it is undergoing it's 3rd or 4th rewrite) and app_server is a lot more capable than it was before we started trying to use it for webkit. So I don't know at which point we should stop efforts in that direction.

/haiku/src/servers/app/DrawState.cpp : DrawState::PenSize returns at least 1 (not in screen space). Even in screen pixel space this would make sense only with antialiasing disabled (no such option in haiku).

There is a B_SUBPIXEL_PRECISE flag on BView to enable full usage of subpixel coordinates. When this flag is set, this method could return smaller values. And indeed the screen vs transformed space issues need to be solved (there are probably more of these)

app_server: Differentiate text and geometry antialiasing generally to prevent rainbow borders on geometry. Because text is usually small and pixel-hinted, artifacts are minimal there.

app_server antialiasing works quite well for native apps I think, so I'm not sure why it looks so bad in WebKit.

The antialiasing on text is also (and mainly) limited by font hinting, which forces the fonts to be aligned closely on subpixel boundaries. There is nothing like that for regular drawing. And I think the Appearance preference to adjust the antialisaing to have more or less colorfringing also only applies to text.

[thebuck]

Point 4. no more observed, assuming fixed by hrev55883. Description updated.

[thebuck]

Regarding 1. An application requests 0.8 px line width...

In BeOS times, Bresenham's algorithm would probably have produced a dashed line (while horizontal and vertical lines would completely disappear depending on their position). Hence, the 0 < linewidth < 1 case had to be avoided.

But with antialiasing (in Haiku) that old application will look just perfect if we removed the exception:
The line would be thinner than now, but it would fit. (I cannot imagine a BeOS app hardcoding 0.01 line width just for fun. As result of a calculation, however, this likely produces a line fitting even better than achievable in BeOS.)

The condition needs an additional AntialiasingIsDisabled && :

	if (penSize < 1.0)
		penSize = 1.0;

in haiku/src/servers/app/DrawState.cpp

Iff that really breaks an app, the user must disable antialiasing ☹ − That's it.

[pulkomandy]

Should the condition be on antialiasing? Or in the B_SUBPIXEL_PRECISE view flag? I think if you want to use pen sizes less than 1, you should set the view with B_SUBPIXEL_PRECISE?

[thebuck]

There are actually two values to decide on:
1.1. what to report back to the application
1.2. what to pass to the renderer

Thoughts about:
1.1. User settings dependent results can only cause confusion here, so better go with B_SUBPIXEL_PRECISE (did not know that). But why at all return a different value than the application set? This only lowers use as storage, ie. set-then-get at different places in code.
1.2. Depends on the underlying renderer only. If it handles 0.x values gracefully in all cases, the exception can be omitted altogether.

[thebuck]

Regarding 3. Add ABI stubs to R1beta4? Enables to deploy improvements immediately in the coming nightly period.

haiku/headers/os/interface/View.h:

void SetLineDashing(float* lengths = nullptr, unsigned lengthsCount = 0, float offset = 0.0);
float LineDashOffset() const;
std::vector<float> LineDashLengths() const;

I do not know the preferred way of exchanging arrays, sorry.

Note that lengthsCount has a compile-time limit of 32 in agg.

[thebuck]

That would pave the way for correctly dotted CSS border & outline as well.
(by using round line caps with [0, 2*linewidth] dashing)

[thebuck]

Quoting pulkomandy (ticket:16101#comment:8):

Adding just the interface will allow building the new haikuwebkit releases with beta4 buildbots, even if they can't display custom fonts on beta4. So I can continue making regular haikuwebkit releases

So, is the ABI I suggested sustainable?

Note that an odd lengthsCount does make sense, causing a pattern period twice as long. The repeated part renders as a negative of the authored pattern then.
However, agg::vcgen_dash does not support this natively; one may pass the lengths list twice, or patch agg to remember dash/gap state independent of m_curr_dash.
Cf. haiku/src/libs/agg/src/agg_vcgen_dash.cpp, line 172:

unsigned cmd = (m_curr_dash & 1) ? path_cmd_move_to : path_cmd_line_to;

[pulkomandy]

It may be too late for beta4 anyway. Let's try to add this API in beta5, that leaves us pleinty of time to figure it out and we can decide to backport it to the beta4 branch or release a beta5 early if we really need it.

Regarding the API, it is quite uncommon to use std::vector (or anything from the STL) in public APIs in Haiku. BList could be used instead, if we want this to be readable at all. It may be a bad idea to make it readable, because that complexifies a few things (for example handling it correctly in BPicture when recording a picture).

Also, I guess it should be affected by PushState/PopState?