Closed
Bug 1176156
Opened 10 years ago
Closed 10 years ago
make sure images are asked to decode if we draw the alt feedback
Categories
(Core :: Layout: Images, Video, and HTML Frames, defect)
Core
Layout: Images, Video, and HTML Frames
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
People
(Reporter: tnikkel, Assigned: tnikkel)
Details
Attachments
(1 file, 1 obsolete file)
1.63 KB,
patch
|
seth
:
review+
|
Details | Diff | Splinter Review |
If we weren't drawing the alt feedback the Draw() call on the image would kick off decoding. But if we do draw the alt feedback we won't Draw() the image and it's possible that nothing will ask it to decode. We should just ask the image to decode if we are drawing the alt feedback for it.
Assignee | ||
Comment 1•10 years ago
|
||
We need a way to ask to decode with async notifications, this is most of this patch.
The patch for bug 1151359 would change this patch, we could call requestDecodeForSize and pass the flags we need. But that wouldn't be quite enough. That patch as written needs mImage (an imgIContainer), but we might not have one yet. We should consider making MaybeDecodeForPredictedSize work without having an image yet.
Attachment #8624608 -
Flags: review?(seth)
Assignee | ||
Comment 2•10 years ago
|
||
This is one of the reason that dynamically inserted images are slow to decode and we show the annoying "loading" placeholder. If the first reflow of the image after it is inserted doesn't place it somewhere that is visible, but a subsequent reflow moves the image to be visible this could happen.
Assignee | ||
Comment 3•10 years ago
|
||
Comment 4•10 years ago
|
||
Comment on attachment 8624608 [details] [diff] [review]
patch
Review of attachment 8624608 [details] [diff] [review]:
-----------------------------------------------------------------
::: image/imgIContainer.idl
@@ +419,5 @@
> /*
> + * Like requestDecode but no notifications are sent during this function call
> + * even if they are already queued for dispatching. No decoding is done either.
> + */
> + void requestDecodeAsyncNotify();
So you could just pass FLAG_ASYNC_NOTIFY to requestDecodeForSize() to get this behavior, right?
::: image/imgIRequest.idl
@@ +157,5 @@
> * container already exists, or calls it once it gets OnStartContainer if the
> * container does not yet exist.
> */
> void requestDecode();
> + void requestDecodeAsyncNotify();
Likewise, this should probably just call requestDecodeForSize() once the image is available.
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Seth Fowler [:seth] from comment #4)
> So you could just pass FLAG_ASYNC_NOTIFY to requestDecodeForSize() to get
> this behavior, right?
Yes, except what size to pass? I don't think there is a way to tell requestDecodeForSize to decode at the intrinsic size and bug 1151359 has the code to compute the size.
After bug 1151359 lands we could do that but we'd want to add requestDecodeForSize to imgRequestProxy and imgRequest so that we can request a decode for a specific size before we have an Image.
Comment 6•10 years ago
|
||
(In reply to Timothy Nikkel (:tn) from comment #5)
> Yes, except what size to pass? I don't think there is a way to tell
> requestDecodeForSize to decode at the intrinsic size and bug 1151359 has the
> code to compute the size.
Yeah, that's true; if we haven't done a size decode yet (so we can't call GetWidth()/GetHeight()), there's a problem. (Actually it should work to just pass (0,0) in that case, since if we don't have a size we just request a decode at the intrinsic size. I don't like that from a style perspective, though, and I actually consider that behavior incorrect, so it'll stop working in the future.)
Now that I think about it though, can't we just call RequestDecode() here? FLAG_ASYNC_NOTIFY only matters if we do a sync decode, which we never will in response to RequestDecode(). I may have missed something, though.
> After bug 1151359 lands we could do that but we'd want to add
> requestDecodeForSize to imgRequestProxy and imgRequest so that we can
> request a decode for a specific size before we have an Image.
Yeah. That's something I want to resolve soon.
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8624608 [details] [diff] [review]
patch
Okay. I'll update the patch to just use RequestDecode.
Attachment #8624608 -
Flags: review?(seth)
Assignee | ||
Comment 8•10 years ago
|
||
Attachment #8624608 -
Attachment is obsolete: true
Attachment #8631807 -
Flags: review?(seth)
Assignee | ||
Comment 9•10 years ago
|
||
Comment 10•10 years ago
|
||
Comment on attachment 8631807 [details] [diff] [review]
patch v2
Review of attachment 8631807 [details] [diff] [review]:
-----------------------------------------------------------------
The patch is so much smaller now. =)
Attachment #8631807 -
Flags: review?(seth) → review+
Comment 11•10 years ago
|
||
Comment 12•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Updated•6 years ago
|
Product: Core → Core Graveyard
Updated•6 years ago
|
Product: Core Graveyard → Core
You need to log in
before you can comment on or make changes to this bug.
Description
•