Open
Bug 511899
Opened 15 years ago
Updated 2 years ago
disable discarding on a per-image basis when it isn't an obvious win
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
NEW
Tracking | Status | |
---|---|---|
blocking2.0 | --- | - |
People
(Reporter: bholley, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
spacer.gif is a good example of this. Should be easy to fix after bug 435296 lands.
Reporter | ||
Updated•15 years ago
|
Summary: disable discarding for images when sizeof(sourceData) > sizeof(decodedFrames) → disable discarding on a per-image basis when it isn't an obvious win
Reporter | ||
Comment 1•15 years ago
|
||
Added a patch for this. I realized that it was probably worth having a threshold of when we want to keep discarding on, since it's probably not worth all the discard overhead just to save 500 bytes. I set this threshold to 8k, but I'm open to suggestions.
Also, I realized that we weren't throwing away source data for non-discardable decode-on-draw images. I was tempted to add that to bug 435296, but I figured it was probably better to do here.
I'll flag for review when bug 435296 settles.
Assignee: nobody → bobbyholley
Status: NEW → ASSIGNED
Reporter | ||
Comment 3•15 years ago
|
||
soon, but not yet.
Reporter | ||
Comment 4•14 years ago
|
||
Just as an update - I actually spent some time working on this again at the beginning of last week.
In order for this to work right currently, the check needs to go in two places, because sometimes we call DecodingCompete() first, and sometimes we call SourceDataCompete() first, and this check needs to happen during the second one, whichever it is.
I talked with joe and jeff about it for a bit, and our eventual conclusion was that it would be better to wait until we fix bug 513681, where we can guarantee consistent decoding behavior across all the decoders and thus only put the check in one place.
Marking the dependency.
Depends on: 513681
This probably should block since it'll fix some annoying behavior with favicons.
blocking2.0: --- → ?
Reporter | ||
Comment 6•14 years ago
|
||
This should be easy to fix for 2.0, since we're already fixing bug 513681. So I agree that it should block.
Updated•14 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Comment 7•14 years ago
|
||
This can't be guaranteed to fix favicon issues.
I think I would release without this fix, but I would take the patch.
blocking2.0: ? → -
Comment 8•14 years ago
|
||
I thought this was supposed to fix the flicker when switching tabs that was introduced when image.mem.discardable was set as default a week or so ago.
Currently to stop the flash/flicker on tab-switch users are setting the pref back to false.
It was turned back on in https://bugzilla.mozilla.org/show_bug.cgi?id=435296 apparently with the knowledge that it would cause flicker, see:
https://bugzilla.mozilla.org/show_bug.cgi?id=435296#c17
The flicker and repaints in forums when switching away from the tab and back after s short period of time is annoying and looks bad, where with the pref disabled there is no flicker or repaints.
So, this bug needs to fix the flicker or bug 435296 backed out.
Comment 9•14 years ago
|
||
With the pref disabled we do not discard and use more memory. The "flickers" you talk about are us redecoding images we have discarded. You didn't see this in Firefox 3.x because we redecoded synchronously, meaning your browser was less responsive.
Reporter | ||
Comment 10•14 years ago
|
||
Let's move this discussion over to bug 516320.
Reporter | ||
Updated•14 years ago
|
Reporter | ||
Updated•13 years ago
|
Assignee: bobbyholley+bmo → nobody
Comment 11•13 years ago
|
||
Joe, do we intend to do anything with this bug? 516320 appears closed.
Comment 12•13 years ago
|
||
This could be an easy win for tiny images. Putting it on memshrink's radar.
Note that, rather than a "minimum win," we should probably check that the source data is "sufficiently" smaller than the decoded data. For example, if the source data is half the size of the decoded data, it's probably not worthwhile to hold on to it.
Whiteboard: [memshrink]
Updated•13 years ago
|
Blocks: image-suck
Whiteboard: [memshrink]
Comment 14•6 years ago
|
||
No assignee, updating the status.
Comment 15•6 years ago
|
||
No assignee, updating the status.
Updated•2 years ago
|
Severity: normal → S3
Comment 16•2 years ago
|
||
The severity field for this bug is relatively low, S3. However, the bug has 12 votes.
:aosmond, could you consider increasing the bug severity?
For more information, please visit auto_nag documentation.
Flags: needinfo?(aosmond)
Comment 17•2 years ago
|
||
The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.
Flags: needinfo?(aosmond)
You need to log in
before you can comment on or make changes to this bug.
Description
•