Closed Bug 1074128 Opened 7 years ago Closed 7 years ago

Add support to Moz2D's AutoSaveTransform for setting the DrawTarget lazily

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(3 files)

In some functions we only set the current transform under certain |if| blocks. In the case that we don't hit those blocks it will give us better performance to avoid restoring the transform that the DrawTarget currently has.
I'd much rather this class were called AutoRestoreTransform.

It irks me that the name is AutoSaveTransform (Save), when the important action that this thing takes is the _restoring_ of the transform (obviously the saving is implicit if it then goes on to restore).
Attachment #8496790 - Flags: review?(bas)
Attachment #8496789 - Flags: review?(bas) → review+
Attachment #8496790 - Flags: review?(bas) → review+
Attachment #8496792 - Flags: review?(bas) → review+
(In reply to Jonathan Watt [:jwatt] from comment #4)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/841bb208fd83
> https://hg.mozilla.org/integration/mozilla-inbound/rev/4f3df060d475
> https://hg.mozilla.org/integration/mozilla-inbound/rev/3b3954b323c6

It would be better to squash part 2 and part 3 into a single commit so that the build is not broken inbetween the two for people doing bisections.
Hmm, Bas specifically likes them separate to make Moz2D merging less of a headache. Is this an issue for regression finding tools? How about I add DONTBUILD to the commit doing the Moz2D changes?
(In reply to Jonathan Watt [:jwatt] from comment #7)
> Hmm, Bas specifically likes them separate to make Moz2D merging less of a
> headache. Is this an issue for regression finding tools? How about I add
> DONTBUILD to the commit doing the Moz2D changes?

Just regular hg bisect or git bisect won't pay attention to DONTBUILD. Bas, can you just filter out the non-moz2d part? What's your workflow for bringing changes into Moz2D?
Flags: needinfo?(bas)
(In reply to Jeff Muizelaar [:jrmuizel] from comment #8)
> (In reply to Jonathan Watt [:jwatt] from comment #7)
> > Hmm, Bas specifically likes them separate to make Moz2D merging less of a
> > headache. Is this an issue for regression finding tools? How about I add
> > DONTBUILD to the commit doing the Moz2D changes?
> 
> Just regular hg bisect or git bisect won't pay attention to DONTBUILD. Bas,
> can you just filter out the non-moz2d part? What's your workflow for
> bringing changes into Moz2D?

I have migration scripts. And I think Moz2D changes should be filtered out. I've suggested better ways of doing this by having a separate repository that's linked to (but I believe there's practical concerns there), or importing changes from Moz2D into central in separate commits. Neither of those were deemed desirable. Without that, this is for me a much preferred approach. Since I think fundamentally a Moz2D change should be considered atomically from the changes in central. Fwiw, there's changesets that don't compile all the time on central (due to bustages with fixing changesets and such), so I don't think it's actually a big issue.
Flags: needinfo?(bas)
Fwiw, I'm fine with doing this in 3 commits, one adding the new function, another changing m-c, another removing the old one from Moz2D. That would be much like what you'd do if you had longer standing migrations to a new API. But it seems a little cumbersome in this case.
You need to log in before you can comment on or make changes to this bug.