Stop calling nsSVGUtils::InvalidateBounds in nsSVGTextFrame2, and use DLBI instead

RESOLVED FIXED in mozilla24



5 years ago
5 years ago


(Reporter: jwatt, Assigned: jwatt)


Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(1 attachment)



5 years ago
We should stop calling nsSVGUtils::InvalidateBounds in nsSVGTextFrame2, and rely on DLBI instead.


5 years ago
Blocks: 873425

Comment 1

5 years ago
Created attachment 751927 [details] [diff] [review]

Passes try with css-frames enabled just fine. Seems like this was copied from nsSVGTextFrame but isn't actually necessary nowadays.
Assignee: nobody → jwatt
Attachment #751927 - Flags: review?(cam)
In bug 839865 you replaced a bunch of InvalidateBounds() calls with SchedulePaint().  What does that do and why do we not need it here?
This is more like bug 802628. We're in ScheduleReflow and not in AttributeChanged so we're going to repaint after this anyway.
Attachment #751927 - Flags: review?(cam) → review+

Comment 4

5 years ago
For DLBI to invalidate, we need two things:

 * we need the refresh driver to have a tick pending after we make our
   changes to the frames (otherwise DLBI won't even be asked to compare
   the old display list/layer tree to what the current display list should
   be, so it won't get the opportunity to invalidate)

 * we need the frames to have actually changed of course, so that when
   DLBI compares the old and current display list and layer tree it picks
   up any changes that have been made

PresShell::DoReflow calls SchedulePaint, so we know if we're under ReflowSVG that a refresh driver tick has been scheduled. This takes care of changes (both style changes and non-style changes) that require a reflow, such as changes to attributes such as "x" (for which we call nsSVGUtils::ScheduleReflowSVG in our AttributeChanged overrides).

As an aside, for style changes a SchedulePaint call is made in DoApplyRenderingChangeToTree as appropriate depending on the change hints that are deduced from the actual changes to the style. This SchedulePaint call exists to take care of repainting for changes that do not trigger a reflow, but it will often also (redundantly, given the call in DoReflow) call SchedulePaint for style change that do result in a reflow.

Note that "transform" attribute changes are an interesting case. These do not schedule reflow and are handled similarly to style changes in that we rely on SVGTransformableElement::GetAttributeChangeHint to include nsChangeHint_UpdateOverflow in the returned hints, and that hint is handled in DoApplyRenderingChangeToTree where we get the SchedulePaint call.

Comment 5

5 years ago
(In reply to Jonathan Watt [:jwatt] from comment #4)
> PresShell::DoReflow calls SchedulePaint, so we know if we're under ReflowSVG
> that a refresh driver tick has been scheduled.

Or in the case that we're under a ReflowSVG call under a NS_STATE_SVG_NONDISPLAY_CHILD tree then we know that we are currently painting, and we don't need to schedule a paint, since it's only the consumers that refer to us that need to paint.
Thanks for the explanation.
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.