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

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

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

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: patch

Passes try with css-frames enabled just fine. Seems like this was copied from nsSVGTextFrame but isn't actually necessary nowadays.

Comment 2

[Cameron McCormack (:heycam)]

In bug 839865 you replaced a bunch of InvalidateBounds() calls with SchedulePaint().  What does that do and why do we not need it here?

Comment 3

[Robert Longson [:longsonr]]

This is more like bug 802628. We're in ScheduleReflow and not in AttributeChanged so we're going to repaint after this anyway.

Comment 4

[Jonathan Watt [:jwatt]]

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

[Jonathan Watt [:jwatt]]

(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.

Comment 6

[Jonathan Watt [:jwatt]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc987f828c72

Comment 7

[Cameron McCormack (:heycam)]

Thanks for the explanation.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dc987f828c72