Load event fires too early for SVG DataURL with DataURL Image




4 years ago
4 years ago


(Reporter: hansschmucker, Unassigned)


(Blocks: 1 bug, {regression, testcase})

34 Branch
Windows 8.1
regression, testcase
Dependency tree / graph

Firefox Tracking Flags

(firefox34 ?, firefox35 ?)




(4 attachments)



4 years ago
Created attachment 8485443 [details]

This issue first appears in tinderbox build 1409452634. It is not present in build 1409416875. It is hard to reproduce across different machines, since it seems to be very timing dependent.

Basically, under some circumstances, the load event on an HTMLImageElement may fire before the SVG document that it's supposed to show has decoded all images. This is significant to me, because it blocks me from reliably applying SVG filters to content that will be rendered back to a Canvas 2D Context.

At the linked URL is a the simplest example that would still exhibit the issue. It's supposed to move one image around while you move the mouse around, then show a copy below with a 200ms delay. It may or may not work for you. Even editing the image seems to make the issue go away for me.

Here are the steps needed to produce the issue:
*Draw an image onto a Canvas
*Draw that Canvas to a second Canvas (with transforms)
*Call toDataURL on that canvas
*Wrap it in a SVG Image within a SVG document (as text)
*Creat an IMG element
*Attach a load handler which would draw the IMG to a third canvas
*Set the SVG document as SRC of the IMG element

Expected result:
*It should reliably render a copy of the image to the third canvas

Actual result:
*Sometimes the image comes out blank.

This might be related to bug #1008942 ... at least that's the only bug linked to landings in that timeframe that seem even vaguely related (changes to http://dxr.mozilla.org/mozilla-central/source/content/base/src/nsImageLoadingContent.cpp ), but since that bug is locked I have no way of knowing.

The issue is really very, very hard to reproduce as it seems very, very timing dependent. Removing the filter tag (which is not applied to anything and empty) from the SVG source fixes the issue. Changing about 3% of the image to different colors fixes the issue. Changing the zoom used while drawing to the second canvas fixes the issue. Changing the delays fixes the issue. Changing the size of the canvas by more than about 20% does. Pretty much any change does.

Firefox was installed in a fresh directory each time with a fresh and different profile. All plugins are disabled, as is any antivirus software. about:support info is attached.

Comment 1

4 years ago
Setting image.mem.decodeondraw AND image.mem.discardable to false also seems to fix the issue.
Could you attach a testcase please rather than just describing the steps.

Comment 3

4 years ago
It's linked at the given URL. Just changed it so that it runs and checks automatically.

Comment 4

4 years ago
Created attachment 8485445 [details]
Just an image used for testing.
(In reply to Hans Schmucker from comment #0)
> This issue first appears in tinderbox build 1409452634. It is not present in
> build 1409416875.

That implies these changesets

Comment 6

4 years ago
Created attachment 8485447 [details]

Testcase. Better use the one at the URL, since that's easier to update, but here's a current snapshot.

Comment 7

4 years ago
Yes, probably due to:
Timothy Nikkel — Bug 1008942. When a network request for an image finishes during paint suppression and the image doesn't have a frame don't start a decode. r=mats Either the image won't get a frame, or it will get a frame very soon and that frame will check its own visibility and kick off a decode if needed.
status-firefox34: --- → ?
status-firefox35: --- → ?
Keywords: regression, testcase
Version: Trunk → 34 Branch
As far as I can tell from the spec (http://www.whatwg.org/specs/web-apps/current-work/multipage/embedded-content.html#the-img-element) the load event only implies that we have received all the data for the image. It doesn't say whether it has to be decoded or not. Now canvas.drawImage should be synchronously decoding the images it needs, and as far as I can tell based on code inspection the sync decode flag should be getting passed through. Maybe we are hitting the canvas image cache or the imagelib surface cache and one of them isn't getting invalidated when it should?

Hans, to try to make this easier to reproduce you could try doing drawImage on an svg image that contains raster images that are never drawn, and there are no references to either image when the document loads. So say add the image dynamically after load with display: none; and then drawImage.

Comment 9

4 years ago
Created attachment 8485494 [details]
Simplified testcase, ~50% accuracy

Added a much simplified testcase... it's not quite as reliable and since the images are not generated dynamically, you'll have to close Firefox and re-start it with a new profile to see the issue after each run (no matter whether it was successful or not... force-reloading and any number of cache settings don't seem to help), but it is a lot simpler.

Comment 11

4 years ago
Just tried it... I can confirm that 1409421612 works (does not produce any errors), while 1409424316 is broken.
So what's happening here is that a decoding thread is in the Decoder::Write call in RasterImage::WriteToDecoder, so mInDecoder is true, and the main thread is in RasterImage::Draw (with the sync decode flag), so we hit the early exit if checking for sync decode and mInDecoder. This is not what the mInDecoder flag is trying to prevent. I think it was created when image decoding was main thread only, so I think it's purpose is to prevent decoding from notifying which causes a Draw when this does more decoding.
Ever confirmed: true
Blocks: 1008942
Depends on: 1079627
Bug 1089046 removes mInDecoder and so should fix this bug.
Depends on: 1089046
Bug 1089046 landed, so this should be fixed now on nightly (version 36).
I just tested nightly and indeed the testcase is now fixed for me. Unfortunately the fix won't make it's way to release until version 36 though unless bug 1089046 gets uplifted, which I don't think there are any plans for at this point.
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.