The default bug view has changed. See this FAQ.

Stop unnecessary propagating COORD_CONTEXT_CHANGED notifications to descendants of nsSVGInnerSVGFrame

RESOLVED FIXED in mozilla15

Status

()

Core
SVG
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

({perf})

Trunk
mozilla15
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
We should stop unnecessary and wastefully propagating COORD_CONTEXT_CHANGED notifications to descendants of nsSVGInnerSVGFrame. Doing so has the potential to cause us to do much more work than necessary, mainly in the form of repainting a much larger area of the screen than we need to.
(Assignee)

Updated

5 years ago
Keywords: perf
(Assignee)

Comment 1

5 years ago
Created attachment 625103 [details] [diff] [review]
patch
Attachment #625103 - Flags: review?(longsonr)
(Assignee)

Comment 2

5 years ago
Actually I've changed the comment to the following locally:

      // Remove COORD_CONTEXT_CHANGED, since we establish the coordinate
      // context for our descendants and this notification won't change its
      // dimensions:

Comment 3

5 years ago
If the inner svg originally had a width or height of 0px and we change to a non-zero px width/height or we have a viewBox and a width/height of 0% and we change to non-zero% width/height then we do need a notification I think as our children won't have had valid user units at all before. Can you add reftests to check each of these cases please and additional logic if necessary.

Comment 4

5 years ago
You might want to clone and adjust the reftests that went wrong when you sent your original patch for bug 413960 to try.
(Assignee)

Comment 5

5 years ago
NotifySVGChanged is never called on a frame for changes to itself. It's called on descendants of the frame that changed. So in your examples, when the width/height/viewBox of the nsSVGInnerSVGFrame change, then NotifySVGChanged will be called on all its descendants without the opportunity for nsSVGInnerSVGFrame::NotifySVGChanged to stop that, since nsSVGInnerSVGFrame::AttributeChanged calls nsSVGUtils::NotifyChildrenOfSVGChange.

Updated

5 years ago
Attachment #625103 - Flags: review?(longsonr) → review+

Comment 6

5 years ago
(In reply to Jonathan Watt [:jwatt] from comment #5)
> NotifySVGChanged is never called on a frame for changes to itself. It's
> called on descendants of the frame that changed. So in your examples, when
> the width/height/viewBox of the nsSVGInnerSVGFrame change, then
> NotifySVGChanged will be called on all its descendants without the
> opportunity for nsSVGInnerSVGFrame::NotifySVGChanged to stop that, since
> nsSVGInnerSVGFrame::AttributeChanged calls
> nsSVGUtils::NotifyChildrenOfSVGChange.

OK, I guess I buy that.
(Assignee)

Comment 7

5 years ago
Pushed https://hg.mozilla.org/integration/mozilla-inbound/rev/17cad7e07453
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/17cad7e07453
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.