Closed
Bug 1082745
Opened 10 years ago
Closed 10 years ago
Avoid reoptimizing optimized SourceSurfaceCairos
Categories
(Core :: Graphics, defect)
Tracking
()
VERIFIED
FIXED
mozilla36
Tracking | Status | |
---|---|---|
firefox33 | --- | wontfix |
firefox34 | + | fixed |
firefox35 | + | fixed |
firefox36 | --- | fixed |
firefox-esr31 | --- | unaffected |
b2g-v1.4 | --- | unaffected |
b2g-v2.0 | --- | unaffected |
b2g-v2.0M | --- | unaffected |
b2g-v2.1 | --- | unaffected |
b2g-v2.2 | --- | unaffected |
People
(Reporter: mwu, Assigned: mwu)
References
Details
(Whiteboard: [MemShrink:P2])
Attachments
(1 file, 1 obsolete file)
919 bytes,
patch
|
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
In some cases, OptimizeSourceSurface will be called on a surface that's already been optimized. This patch makes the function return early in that case.
Attachment #8504923 -
Flags: review?(bas)
Updated•10 years ago
|
Whiteboard: [MemShrink]
Comment 1•10 years ago
|
||
Comment on attachment 8504923 [details] [diff] [review] Avoid reoptimizing optimized SourceSurfaceCairos Review of attachment 8504923 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/DrawTargetCairo.cpp @@ +1277,5 @@ > #ifdef CAIRO_HAS_XLIB_SURFACE > + if (aSurface->GetType() == SurfaceType::CAIRO && > + cairo_surface_get_type( > + static_cast<SourceSurfaceCairo*>(aSurface)->GetSurface()) == > + CAIRO_SURFACE_TYPE_XLIB) { We could probably make this generic and compare the type of aSurface to that of mSurface. That'd make it cleaner if we ever implement optimizing for other cairo backends.
Updated•10 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Comment 2•10 years ago
|
||
Comment on attachment 8504923 [details] [diff] [review] Avoid reoptimizing optimized SourceSurfaceCairos Review of attachment 8504923 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/DrawTargetCairo.cpp @@ +1277,5 @@ > #ifdef CAIRO_HAS_XLIB_SURFACE > + if (aSurface->GetType() == SurfaceType::CAIRO && > + cairo_surface_get_type( > + static_cast<SourceSurfaceCairo*>(aSurface)->GetSurface()) == > + CAIRO_SURFACE_TYPE_XLIB) { Agreed.
Attachment #8504923 -
Flags: review?(bas) → review+
Assignee | ||
Comment 3•10 years ago
|
||
The check is now generic.
Attachment #8504923 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Hardware: x86_64 → All
Assignee | ||
Comment 4•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f013910bbead
Assignee | ||
Comment 5•10 years ago
|
||
[Tracking Requested - why for this release]: This fixes severe leaks for certain canvas users (like freeciv) on GTK/X11.
status-b2g-v1.4:
--- → unaffected
status-b2g-v2.0:
--- → unaffected
status-b2g-v2.0M:
--- → unaffected
status-b2g-v2.1:
--- → unaffected
status-b2g-v2.2:
--- → unaffected
status-firefox34:
--- → affected
status-firefox35:
--- → affected
status-firefox36:
--- → affected
status-firefox-esr31:
--- → unaffected
tracking-firefox34:
--- → ?
tracking-firefox35:
--- → ?
tracking-firefox36:
--- → ?
Comment 6•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f013910bbead
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 7•10 years ago
|
||
(In reply to Michael Wu [:mwu] from comment #5) > [Tracking Requested - why for this release]: > This fixes severe leaks for certain canvas users (like freeciv) on GTK/X11. How far back does this issue go?
Flags: needinfo?(mwu)
Assignee | ||
Comment 8•10 years ago
|
||
Goes back to Gecko 32 when bug 994081 landed.
Flags: needinfo?(mwu)
Assignee | ||
Updated•10 years ago
|
Comment 9•10 years ago
|
||
Thanks for the bug ref. Once this has been verified on m-c, let's get it uplifted to Beta and Aurora.
status-firefox33:
--- → wontfix
tracking-firefox36:
? → ---
Assignee | ||
Comment 10•10 years ago
|
||
To verify, use a Linux system/VM, and load freeciv from the link in bug 1081926 - http://play.freeciv.org/webclient/?action=load&load=tutorial&scenario=true . Watch Xorg in top (or your favorite process monitor) and make sure the memory usage doesn't jump several gigabytes when freeciv loads.
Keywords: verifyme
Comment 11•10 years ago
|
||
(In reply to Michael Wu [:mwu] from comment #10) > To verify, use a Linux system/VM, and load freeciv from the link in bug > 1081926 - > http://play.freeciv.org/webclient/?action=load&load=tutorial&scenario=true . > Watch Xorg in top (or your favorite process monitor) and make sure the > memory usage doesn't jump several gigabytes when freeciv loads. But note that it doesn't affect all systems. I was unable to reproduce on my Ubuntu 14.04 box.
Assignee | ||
Comment 12•10 years ago
|
||
Xrender needs to be enabled/supported to trigger this bug.
Assignee | ||
Comment 13•10 years ago
|
||
Verified as fixed by user on bug 1081926 comment 22 .
Status: RESOLVED → VERIFIED
Keywords: verifyme
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8505586 [details] [diff] [review] Avoid reoptimizing optimized SourceSurfaceCairos, v2 Approval Request Comment [Feature/regressing bug #]: Regressed by bug 994081. [User impact if declined]: Using the canvas API in certain ways will cause massive memory usage in the X server on GTK/X11 systems. It can often cause an X server OOM crash. [Describe test coverage new/current, TBPL]: No tests, but testing this is somewhat difficult since it'll be made ineffective once bug 1081926 is fixed. [Risks and why]: Low. I can't think of any risks. [String/UUID change made/needed]: None.
Attachment #8505586 -
Flags: approval-mozilla-beta?
Attachment #8505586 -
Flags: approval-mozilla-aurora?
Comment 15•10 years ago
|
||
Comment on attachment 8505586 [details] [diff] [review] Avoid reoptimizing optimized SourceSurfaceCairos, v2 Beta+ Aurora+
Attachment #8505586 -
Flags: approval-mozilla-beta?
Attachment #8505586 -
Flags: approval-mozilla-beta+
Attachment #8505586 -
Flags: approval-mozilla-aurora?
Attachment #8505586 -
Flags: approval-mozilla-aurora+
Comment 16•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/fdd0036a0ae5 https://hg.mozilla.org/releases/mozilla-beta/rev/5e3fc9d8a99b
You need to log in
before you can comment on or make changes to this bug.
Description
•