We were transforming coordinates for SVG gradients in a pretty
convoluted way: an inverse, unscaled transformation matrix was set up in
order to work around some (old?) technical limitations.
Rework this so the coordinate transformation no longer needs to be
inversed. This fixes gradients with "userSpaceOnUse" for its
gradientUnits attribute, which might cause coordinates to lie outside of
the bounding box of the gradient.
Two tests have updated reference screenshots with minor pixel updates;
this is probably the result of floating point precision improvements by
not inversing the matrix.
One test (svg-text-effects) has a bigger change: the gradient stops seem
to have moved along the text. This does seem to match other browsers
slightly better, so I'm moving forward with this ref update.
...and add display list item that does translation instead. By doing
that we no longer need to map each coordinate in display list by
translation in recorder state.
Prior to this change, SVGs were following the CSS painting order, which
means SVG boxes could have established stacking context and be sorted by
z-index. There is a section in the spec that defines what kind of SVG
boxes should create a stacking context
https://www.w3.org/TR/SVG2/render.html#EstablishingStackingContex
Although this spec is marked as a draft and rendering order described in
this spec does not match what other engines do.
This spec issue comment has a good summary of what other engines
actually do regarding painting order
https://github.com/w3c/svgwg/issues/264#issuecomment-246432360
"as long as you're relying solely on the default z-index (which SVG1
does, by definition), nothing ever changes order when you apply
opacity/filter/etc".
This change aligns our implementation with other engines by forbidding
SVGs to create a formatting context and painting them in order they are
defined in tree tree.
Instead of converting images to alpha masks on the CPU, we now delegate
that work to the GPU if possible, by way of SkSL shaders.
This noticeably speeds up https://vercel.com/ which has a ton of SVG
masking going on. The old implementation used 15% of CPU time when
loading the page, this one uses basically none.
It would be nice if we could somehow move this work to the GPU, but even
with some basic local optimization (mostly coalescing bounds checks and
inlining pixel data access), this knocks a 13% item down to 9% in a
profile of loading https://vercel.com/
Adjust the translation from Gfx::ScalingMode to Skia SkFilterMode, so
that CSS::ImageRendering::Pixelated will result in
SkFilterMode::kNearest.
Before:
ScalingMode::SmoothPixels -> kLinear
After:
ScalingMode::SmoothPixels -> kNearest
Fixes#1170
Instead, it could be applied directly as a clip path in Skia painter.
As a side bonus, we get rid of some DeprecatedPath and
AntiAliasingPainter usage.
SVG and and CSS border rendering now sits on top of SkPath instead of
the old Gfx::DeprecatedPath.
Due to an imperceptible (255, 255, 255) vs (255, 254, 255) color diff
in one ref test, I changed that test to not depend on border rendering
for a positive result, since that was incidental.
A new display list item type named PaintScrollBar is introduced. Having
a dedicated type for scroll bars allows the thumb position to be updated
without rebuilding a display list. This was not possible with
FillRectWithRoundedCorners that does not allow to tell whether it
belongs to scroll thumb.
While introducing clip and scroll frame trees, I made a mistake by
assuming that the paintable tree includes boxes from nested navigables.
Therefore, this comment in the code was incorrect, and clip/scroll
frames were simply not assigned for iframes:
// NOTE: We only need to refresh the scroll state for traversables
// because they are responsible for tracking the state of all
// nested navigables.
As a result, anything with "overflow: scroll" is currently not
scrollable inside an iframe
This change fixes that by ensuring clip and scroll frames are assigned
and refreshed for each navigable. To achieve this, I had to modify the
display list building process to record a separate display list for each
navigable. This is necessary because scroll frame ids are local to a
navigable, making it impossible to call
`DisplayList::apply_scroll_offsets()` on a display list that contains
ids from multiple navigables.
When converting a `Gfx::Bitmap` to a Skia bitmap, we cannot assume the
color data is unpremultiplied. For example, everything canvas-related
uses premultiplied color data:
https://html.spec.whatwg.org/multipage/canvas.html#premultiplied-alpha-and-the-2d-rendering-context
We were probably assuming unpremultiplied since that is what the PNG
decoder gives us. Since we now make `Gfx::Bitmap` identify what alpha
type is being used, we can instruct Skia a bit better :^)
Update our `EdgeFlagPathRasterizer` to use premultiplied alpha instead
of unpremultiplied so we can apply alpha correctly for path masks.
This fixes the dark borders sometimes visible when SVGs are blended
with a colored background.
This also exposed an issue with our `CanvasRenderingContext2D`, which is
supposed to hold a bitmap with premultiplied alpha internally but expose
a bitmap with unpremultiplied alpha in `CanvasImageData`. Expand our C2D
test to include the alpha channel as well.
Finally, this also exposed an off-by-one issue in
`EdgeFlagPathRasterizer` which caused the last scanlines for edges to
render incorrectly. We had some reference images which included these
corruptions (they were almost unnoticeable), so update them as well.
Instead of carrying the display list for a mask in each command that
might potentially be affected by "background-clip: text", this change
introduces a new AddMask command that is applied once for all
background layers within one box.
The new AddMask command includes a rectangle for the mask destination
that is translated by the corresponding scroll offset.
Fixes https://github.com/LadybirdBrowser/ladybird/issues/857
Contrary to LibGfx, where corner clipping was implemented by sampling
and blitting pixels under corners into a temporary bitmap, Skia allows
us to simply apply a mask. As a result, we no longer need the
BlitCornerClipping command, which has become a no-op.
- SampleUnderCorners is renamed to AddRoundedRectClip
- The optimization that skipped unnecessary blit and sample commands has
been removed. However, this should not result in a performance
regression because Skia seems to perform mask rasterization lazily.
The main incentive is much better performance. We could have gone a bit
further in optimizing the Skia painter to blit glyphs produced by LibGfx
more efficiently from the glyph atlas, but eventually, we also want Skia
to improve correctness.
This change does not completely replace LibGfx in text handling. It's
still used at all stages, including layout, up until display list
replaying.
Before this change, "background-clip: text" was implemented by saving a
Vector<Gfx::Path> of all glyphs needed to paint a mask for the
background. The issue with this approach was that once glyphs were
extracted into vector paths, the glyph rasterization cache could no
longer be utilized.
With this change, all text required for mask painting is saved in a
nested display list and rasterized as a regular text.
CommandResult was needed by LibGfx display list player that could have
failed to allocate a temporary bitmap for painting a stacking context
with CSS transforms. This is no longer an issue with Skia painter, so
we can delete CommandResult::SkipStackingContext handling path.
With this change, instead of recording a display list item for each
instance of a repeated background, a new DrawRepeatedImmutableBitmap
type is used. This allows the painter to use optimized repeated image
painting and, when the GPU backend is used, avoid re-uploading the image
texture for each repetition.
Some screenshot tests are affected, but there are no visible
regressions.
https://null.com/games/chainstaff works a lof faster with this change.
If no header includes the prototype of a function, then it cannot be
used from outside the translation unit it was defined in. In that case,
it should be marked as `static`, in order to avoid possible ODR
problems, unnecessary exported symbols, and allow the compiler to better
optimize those.
If this warning triggers in a function defined in a header, `inline`
needs to be added, otherwise if the header is included in more than one
TU, it will fail to link with a duplicate definition error.
The reason this diff got so big is that Lagom-only code wasn't built
with this flag even in Serenity times.
Each item in clip_paths represents a glyph run, and applying them as a
clip in intersection mode one by one results in an empty clip. Instead,
now all clip paths are joined and applied as a clip together.
This change fixes rendering of "background-clip: text" when an element
has more than one glyph run.
Fixed ref-test: Tests/LibWeb/Ref/css-background-clip-text.html
Couple fixes found by reading the spec:
- Repeating should also happen in negative direction, so the whole
[0, 1] is covered.
- Leftmost and rightmost stops should be clamped to [0, 1] range if
needed, because Skia ignores everything outside of this range.