Hack around Bug 695763 for drawWindow

RESOLVED WONTFIX

Status

()

Core
Canvas: 2D
RESOLVED WONTFIX
6 years ago
6 years ago

People

(Reporter: khuey, Assigned: khuey)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 obsolete attachments)

Comment hidden (empty)
Created attachment 568133 [details] [diff] [review]
Patch

Force sync decoding of everything in the document's image tracker and in subdocuments.
Attachment #568133 - Flags: review?(bzbarsky)
Comment on attachment 568133 [details] [diff] [review]
Patch

The GetOurOwnerDoc change seems out of place here....

The succeeded setup will mean that drawWindow on any document with SVG images will warn.  That doesn't seem desirable.  Should VectorImage just return NS_OK?

This probably needs review from an imagelib person too.
(In reply to Boris Zbarsky (:bz) from comment #2)
> Comment on attachment 568133 [details] [diff] [review] [diff] [details] [review]
> Patch
> 
> The GetOurOwnerDoc change seems out of place here....

Yeah, the hazards of having 20 patches applied :-/

> The succeeded setup will mean that drawWindow on any document with SVG
> images will warn.  That doesn't seem desirable.  Should VectorImage just
> return NS_OK?

I think this is the desired behavior until somebody hooks up synchronous decoding for VectorImage.  I'm just implementing enough here to make reftests pass with the patch from bug 685516 ...

> This probably needs review from an imagelib person too.

Yeah.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #3)
> (In reply to Boris Zbarsky (:bz) from comment #2)
> > Comment on attachment 568133 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]
> > The succeeded setup will mean that drawWindow on any document with SVG
> > images will warn.  That doesn't seem desirable.  Should VectorImage just
> > return NS_OK?
> 
> I think this is the desired behavior until somebody hooks up synchronous
> decoding for VectorImage.  I'm just implementing enough here to make
> reftests pass with the patch from bug 685516 ...

Actually, we should do something better here.   Not sure what ...

Maybe NS_OK is ok for now.
We should probably return NS_OK if VectorImage::mFullyLoaded is true, and NS_ERROR_FAILURE otherwise.  Does that seem reasonable?
That seems good to me.
Created attachment 568161 [details] [diff] [review]
Patch

Comments addressed
Attachment #568133 - Attachment is obsolete: true
Attachment #568133 - Flags: review?(bzbarsky)
Attachment #568161 - Flags: review?(bzbarsky)
Comment on attachment 568161 [details] [diff] [review]
Patch

r=me; again get some imagelib person to look too.
Attachment #568161 - Flags: review?(bzbarsky) → review+
Attachment #568161 - Flags: review?(jmuizelaar)
Comment on attachment 568161 [details] [diff] [review]
Patch

Not beautiful, but seems ok. I'd appreciate a comment in the idl about why SyncDecode is being exposed.

I've also pinged bholley for review, but I think it's fine to push this before he responds.
Attachment #568161 - Flags: review?(jmuizelaar)
Attachment #568161 - Flags: review?(bobbyholley+bmo)
Attachment #568161 - Flags: review+
So Image::Draw has a SYNC_DECODE flag, would using that be sufficient for solving this bug?
Comment on attachment 568161 [details] [diff] [review]
Patch

So, I'm not really excited about this patch.

If the problem is discarding, the retained layer should already contain what we want. If the problem is that the image is mid-decode, the invalidations should trigger us to re-draw the layer, which we should do with the SYNC_DECODE flag to Draw(). Which problem are we trying to solve here?

r- for now until we talk about it some more.

(In reply to Jeff Muizelaar [:jrmuizel] from comment #9)
> I've also pinged bholley for review, but I think it's fine to push this
> before he responds.

Blasphemy!
Attachment #568161 - Flags: review?(bobbyholley+bmo) → review-
Comment on attachment 568161 [details] [diff] [review]
Patch

This patch doesn't work anyways :-(
Attachment #568161 - Attachment is obsolete: true
> Which problem are we trying to solve here?

1)  We're mid-decode.
2)  We try to drawWindow onload, which requires that the decode be done by then.
3)  Image decodes for lots of images don't block onload.

#2 is why invalidations don't help.  #1 is why discarding is not the issue.

Your options are to fix item 3 above or to have a way to force sync decode on drawWindow.
Going to do Bug 697230 instead.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.