While 788d5368a7 took care of better text
marker positioning, this improves graphical marker positioning instead.
By looking at how Firefox and Chrome render markers, it's clear that
there are three parts to positioning a graphical marker:
* The containing space that the marker resides in;
* The marker dimensions;
* The distance between the marker and the start of the list item.
The space that the marker can be contained in, is the area to the left
of the list item with a height of the marker's line-height. The marker
dimensions are relative to the marker's font's pixel size: most of them
are a square at 35% of the font size, but the disclosure markers are
sized at 50% instead. Finally, the marker distance is always gauged at
50% of the font size.
So for example, a list item with `list-style-type: disc` and `font-size:
20px`, has 10px between its start and the right side of the marker, and
the marker's dimensions are 7x7.
The percentages I've chosen closely resemble how Firefox lays out its
list item markers.
`CSSColorValue`s which have unresolved `calc` components should be able
to be resolved. Previously we would always resolve them but with
incorrect values.
This is useful as we will now be able to now whether we should serialize
colors in their normalized form or not.
Slight regression in that we now serialize (RGB, HSL and HWB) colors
with components that rely on compute-time information as an empty
string, but that will be fixed in the next commit.
Until now, every paint phase of every PaintableBox injected its own
clipping sequence into the display list:
```
before_paint: Save
AddClipRect (1)
...clip rectangles for each containing block with clip...
AddClipRect (N)
paint: ...paint phase items...
after_paint: Restore
```
Because we ran that sequence for every phase of every box, Skia had to
rebuild clip stack `paint_phases * paintable_boxes` times. Worse,
usually most paint phases contribute no visible drawing at all, yet we
still had to emit clipping items because `before_paint()` has no way to
know that in advance.
This change takes a different approach:
- Clip information is now attached as metadata `ClipFrame` to each
DisplayList item.
- `DisplayListPlayer` groups consecutive commands that share a
`ClipFrame`, applying the clip once at the start of the group and
restoring it once at the end.
Going from 10 ms to 5 ms in rasterization on Discord might not sound
like much, but keep in mind that for 60fps we have 16 ms per frame and
there is a lot more work besides display list rasterization we do in
each frame.
* https://discord.com/channels/1247070541085671459/1247090064480014443
- DisplayList items: 81844 -> 3671
- rasterize time: 10 ms -> 5 ms
- record time: 5 ms -> 3 ms
* https://github.com/LadybirdBrowser/ladybird
- DisplayList items: 7902 -> 1176
- rasterize time: 4 ms -> 4 ms
- record time: 3 ms -> 2 ms
In the upcoming change, device pixel conversion of ClipFrame will
happen during display list replay, where PaintContext is not available,
so let’s move it out of PaintContext.
It's useful to have tests that dump display list items, so we can more
easily see how changes to the display list recording process affect the
output. Even the small sample test added in this commit shows that we
currently record an unnecessary AddClipRect item for empty paint phases.
For now, the dump doesn't include every single property of an item, but
we can shape it to include more useful information as we iterate on it.
Recently, we moved the backing store manager into Navigable, which means
we now try to allocate a backing store for all navigables, including
those corresponding to SVG image documents. This change disables that
behavior for all navigables except top-level non-SVG traversables,
because otherwise it causes issues when we stop repainting: the browser
process was notified about an allocated backing stores that does not
correspond to the page, and then all subsequent repaints are ignored
until the window is resized.
83b6bc4 went too far by forbidding SVGSVGElement from establishing a
stacking context. This element type does follow the behavior of CSS
boxes, unlike inner SVG elements like `<rect>`, `<circle>`, etc., which
are not supposed to be aware of concepts like stacking contexts,
overflow clipping, scroll offsets, etc.
This change allows us to delete overrides of `before_paint()` and
`after_paint()` in SVGPaintable and SVGSVGPaintable, because display
list recording code has been rearranged to take care of clipping and
scrolling before recursing into SVGSVGPaintable descendants.
`Screenshot/images/css-transform-box-ref.png` expectation is updated and
fixes a bug where a rectangle at the very bottom of the page was not
clipped correctly.
`Screenshot/images/svg-filters-lb-website-ref.png` has a more subtle
difference, but if you look closely, you’ll see it matches other
browsers more closely now.
By avoiding recompilation every time `apply_mask_bitmap()` is called, we
save ~5 ms (20ms -> 15ms) in rendering of browser channel on Discord on
my machine.
Cuts display list size, mostly because now we avoid lots of FillRect
previusly recorded for boxes with transparent background.
Website | DisplayList Items Before | DisplayList Items After
-------------|--------------------------|-------------------------
ladybird.org | 1431 | 1117
null.com | 4714 | 4484
discord.com | 5360 | 4992
`paint_background()` is invoked for each PaintableBox, so by avoiding
save/restore pair emitted for each call, we substantially decrease
display list size.
Website | DisplayList Items Before | DisplayList Items After
-------------|--------------------------|-------------------------
ladybird.org | 2753 | 1431
null.com | 5298 | 4714
discord.com | 6598 | 5360
Initially ClippableAndScrollable was introduced, because we had
PaintableBox and InlinePaintable and both wanted to share clipping and
scrolling logic. Now, when InlinePaintable is gone, we could inline
ClippableAndScrollable implementation into PaintableBox.
Previously, we always applied the enclosing clip rectangle for all paint
phases except overlays, and the own clip rectangle for the background
and foreground phases. The problem is that applying a clip rectangle
means emitting an AddClipRect display list item for each clip rectangle
in the containing block. With this change, we choose whether to include
the own clip based on the paint phase and this way avoid emitting
AddClipRect for enclosing clip rectangles twice.
Own clip rect is alredy applied in `PaintableBox::before_paint()` for
all paintables with lines, so there's no need to do it once again in
`PaintableWithLines::paint()`.
Own clip rect is alredy applied in `PaintableBox::before_paint()` for
all image paintables, so there's no need to do it once again in
`ImagePaintable::paint()`.
Unbalanced save and restore means that effects only relevant to a
stacking context leak outside, which is never expected behavior. Having
a `VERIFY()` for that makes it much easier to catch such issues.
These were only used in SVGSVGPaintable to apply scroll frame id, which
is already handled by `before_paint()` and `after_paint()` hooks in
PaintableBox.
Unbalanced save/restore within display list items recorded for a
paintable means that some state only relevant for the paintable leaks to
subsequent paintables, which is never expected behavior.
We were always delegating hit tests to PaintableBox if a
PaintableWithLines has no fragments, which means that anonymous
containers could overlap with previous siblings and prioritize their
border box rect. Instead, the nearest non-anonymous ancestor should take
care of hit testing the children so the correct order is maintained.
To achieve this, we no longer do an early hit test in
PaintableWithLines::hit_test() if there are no fragments and default
to the later PaintableBox::hit_test() call that does take anonymous
containers into account.
Fixes the issue seen in #4864.
This way we can reuse the logic between PaintableWithLines and
PaintableBox. It also introduces the .is_positioned() check for the
children of a PaintableWithLines, which makes sure to skip positioned
child nodes since those are handled by the StackingContext.
A PaintableWithLines will first try to see if there are any fragments
that have a hit. If not, it falls back to hit testing against its border
box rect.
However, inline content is often hoisted out of its parent into an
anonymous container to maintain the invariant that all layout nodes
either have inline or block level children. If that's the case, we
should not check the border box rect of the anonymous container, because
we might trigger a hit too early if the node has previous siblings in
its original parent node that overlap with their bounds.
By ignoring anonymous nodes, we leave the border box hit testing to the
nearest non-anonymous ancestor, which correctly applies the hit testing
order to its children.
Note that the border box rect checks whether the _untransformed_ point
is inside of it, which mirrors the behavior of PaintableBox::hit_test().
Now, when Skia backend context is available by the time backing stores
are allocated, there is no need to have a separate BackingStore class.
This allows us to get rid of BackingStore -> PaintingSurface cache.
Making navigables responsible for backing store allocation will allow us
to have separate backing stores for iframes and run paint updates for
them independently, which is a step toward isolating them into separate
processes.
Another nice side effect is that now Skia backend context is ready by
the time backing stores are allocated, so we will be able to get rid of
BackingStore class in the upcoming changes and allocate PaintingSurface
directly.
This change fixes at least two issues:
- `update_associated_selection()` is responsible for selectionchange
dispatch, and we were incorrectly dispatching this event on ranges
that were not associated with a selection.
- `Range::get_client_rects()` was using `update_associated_selection()`
to refresh the selection state in the paintable tree for the current
range, but since a range might not be associated with a selection,
this could make the painted selection reflect the state of an
arbitrary range instead of the actual selection range.
Fixes a bug on Discord where any text typed into the message input would
get selected.
This check technically isn't necessary in
`SVGForeignObjectPaintable::paint()` because
`PaintableWithLines::paint(context, phase);` does the check already, but
I've added it there anyway to save some debugging time if someone does
add more code there. :^)
We were calculating the scroll delta based on the last known mouse
position, even if that position ventured far beyond the scroll bar's
rect. This caused weird behavior such as scrolling when the mouse was
clearly not on the scrollbar.
This reworks the scrollbar logic to use the mouse's absolute position
instead of a delta, and to always calculate and set the absolute scroll
offset. This makes it much easier to reason about the scrollbar's
position, plus we don't need the last mouse position anymore.
As far as I can tell, our scroll bar behavior now closely resembles
Firefox' behavior.
For every invocation of `::before_paint()` and `::after_paint()`, we
would reach into the node's computed values to determine its visibility.
Let's just do this once during construction of the paintable instead,
since this was showing up in profiles.
