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.
In `StackingContext::paint_descendants()`, we don't need to obtain the
computed values nor the Z-index of a child unless certain other
conditions are true. Let these conditions short-circuit before actually
reaching into the computed values, which shows up in profiles.
We generated `PaintableFragment`s with a start and length represented in
UTF-8 byte offsets, but failed to consider that the offsets in a
`DOM::Range` are actually expressed in UTF-16 code units.
This is a bit of a mess: almost all web specs use UTF-16 code units as
the unit for indexing into text nodes, but we almost exclusively use
UTF-8 in our code base. Arguably the best thing would for us to use
UTF-16 everywhere as well: it prevents these mismatches in our
implementations for the price of a bit more memory usage - and even that
could potentially be optimized for.
But for now, try to do the correct thing and lazily allocate UTF-16 data
in a `PaintableFragment` whenever we need to index into it or if we're
asked to determine the code unit offset of a pixel position.
When clicking on a glyph or starting a selection on it, we would use the
glyph's offset/index as the position which represents the left side of
the glyph, or the position between the glyph and the glyph before it.
Instead of looking at which glyph is under the mouse pointer, look at
which glyph boundary is closer. Now, if you click to the right of a
glyph (but still on that glyph), it correctly selects the next glyph's
offset as the position.
Instead, collect a list of all the elements with content-visibility:auto
after layout.
This way we can skip the tree traversal when updating the rendering.
This was previously eating up ~300 µs of the 60fps frame budget on
our GitHub repo pages (and even more on large pages).
