695765 - Hack around Bug 695763 for drawWindow

Status: RESOLVED WONTFIX

(Core :: Graphics: Canvas2D, defect)

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Force sync decoding of everything in the document's image tracker and in subdocuments.

Comment 2

[Boris Zbarsky [: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.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

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

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Kyle Huey [:khuey] ([e-mail]) 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.

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

We should probably return NS_OK if VectorImage::mFullyLoaded is true, and NS_ERROR_FAILURE otherwise.  Does that seem reasonable?

Comment 6

[Boris Zbarsky [:bzbarsky]]

That seems good to me.

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comments addressed

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 568161 [details] [diff] [review]
Patch

r=me; again get some imagelib person to look too.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

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.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

So Image::Draw has a SYNC_DECODE flag, would using that be sufficient for solving this bug?

Comment 11

[Bobby Holley (:bholley)]

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!

Comment 12

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 568161 [details] [diff] [review]
Patch

This patch doesn't work anyways :-(

Comment 13

[Boris Zbarsky [:bzbarsky]]

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

Comment 14

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Going to do Bug 697230 instead.