CanvasRenderingContext2D::Demote should preserve DrawTarget state

RESOLVED FIXED in mozilla26

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: roc, Assigned: snorp)

Tracking

Trunk
mozilla26
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

The DrawTarget's transform and clip need to be copied to the new DrawTarget.
An interesting case is when we push a few clips, Demote(), then pop a clip or two.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #1)
> An interesting case is when we push a few clips, Demote(), then pop a clip
> or two.

I kinda wonder if we should add a mozDemote() method or something so we can explicitly demote in order to test this thing.
Created attachment 793111 [details] [diff] [review]
Restore the DrawTarget transform and clip when demoting

This also includes a mochitest and some machinery for demoting canvases from there
Attachment #793111 - Flags: review?(matt.woodrow)
I don't really like the event hack. I think I'm going to add a method to CanvasRenderingContext2D.idl instead.
Created attachment 793222 [details] [diff] [review]
better patch
Attachment #793222 - Flags: review?(matt.woodrow)
Attachment #793111 - Attachment is obsolete: true
Attachment #793111 - Flags: review?(matt.woodrow)
Comment on attachment 793222 [details] [diff] [review]
better patch

Review of attachment 793222 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, please find someone else to review the .webidl change though.

We still the actual copying of state patch too, right?
Attachment #793222 - Flags: review?(matt.woodrow) → review+
Attachment #793222 - Flags: review?(bzbarsky)
Created attachment 793489 [details] [diff] [review]
Restore the DrawTarget transform and clip when demoting

Indeed I forgot the part to actually restore the clip/transform was in a different patch. Fixed.
Attachment #793222 - Attachment is obsolete: true
Attachment #793222 - Flags: review?(bzbarsky)
Attachment #793489 - Flags: review?(matt.woodrow)
Attachment #793489 - Flags: review?(bzbarsky)
Comment on attachment 793489 [details] [diff] [review]
Restore the DrawTarget transform and clip when demoting

>+#if USE_SKIA_GPU

ifdef, right?

r=me on the binding part, assuming that was what you wanted me to review.  If it wasn't, please say what you _did_ want me to review.
Attachment #793489 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky [:bz] from comment #8)
> Comment on attachment 793489 [details] [diff] [review]
> Restore the DrawTarget transform and clip when demoting
> 
> >+#if USE_SKIA_GPU
> 
> ifdef, right?

Yup, good catch.

> 
> r=me on the binding part, assuming that was what you wanted me to review. 
> If it wasn't, please say what you _did_ want me to review.

Yes, that was the bit I wanted you to review. Thanks!
Comment on attachment 793489 [details] [diff] [review]
Restore the DrawTarget transform and clip when demoting

Review of attachment 793489 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/canvas/src/CanvasRenderingContext2D.h
@@ +567,5 @@
>  
>      return CurrentState().font;
>    }
>  
>  #if USE_SKIA_GPU

#ifdef here too
Attachment #793489 - Flags: review?(matt.woodrow) → review+
(I fixed up all the s/if/ifdef/ stuff, thanks)
https://hg.mozilla.org/mozilla-central/rev/d76df67ac3b1
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.