Closed
Bug 1060790
Opened 10 years ago
Closed 10 years ago
Call glFlush() to SkiaGL's GLContext on memory-pressure
Categories
(Core :: Graphics, defect)
Tracking
()
People
(Reporter: sotaro, Assigned: sotaro)
References
Details
(Whiteboard: [MemShrink:P2])
Attachments
(1 file, 1 obsolete file)
818 bytes,
patch
|
sotaro
:
review+
bajaj
:
approval-mozilla-aurora+
bajaj
:
approval-mozilla-b2g32+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1049195 +++
During investigating Bug 1049195, I recognized that glFush()/glFinish() works to reduce kgls memory usage. But on memory-pressure, glFlush() is not called to SkiaGL's GLContext.
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8481780 -
Flags: review?(gwright)
Assignee | ||
Comment 2•10 years ago
|
||
[Blocking Requested - why for this release]: Blocking 2.0+ bugs.
blocking-b2g: --- → 2.0?
Comment 3•10 years ago
|
||
Comment on attachment 8481780 [details] [diff] [review]
patch - Call glFlush() to SkiaGL's GLContext on memory-pressure
Review of attachment 8481780 [details] [diff] [review]:
-----------------------------------------------------------------
Why not call GrContext::flush()?
Updated•10 years ago
|
Whiteboard: [MemShrink]
Updated•10 years ago
|
blocking-b2g: 2.0? → 2.0+
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to George Wright (:gw280 PTO Aug 15th - 24th) from comment #3)
> Comment on attachment 8481780 [details] [diff] [review]
> patch - Call glFlush() to SkiaGL's GLContext on memory-pressure
>
> Review of attachment 8481780 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Why not call GrContext::flush()?
GrContext::flush() is called in GrContext::freeGpuResources(). But GrContext::flush() does not call glFlush().
http://mxr.mozilla.org/mozilla-central/source/gfx/skia/trunk/src/gpu/GrContext.cpp#216
Assignee | ||
Comment 5•10 years ago
|
||
GrContext::flush() calls GrInOrderDrawBuffer::flush().
http://mxr.mozilla.org/mozilla-central/source/gfx/skia/trunk/src/gpu/GrInOrderDrawBuffer.cpp#556
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #5)
> GrContext::flush() calls GrInOrderDrawBuffer::flush().
> http://mxr.mozilla.org/mozilla-central/source/gfx/skia/trunk/src/gpu/
> GrInOrderDrawBuffer.cpp#556
GrInOrderDrawBuffer::flush() does not call glFlush().
Comment 7•10 years ago
|
||
Comment on attachment 8481780 [details] [diff] [review]
patch - Call glFlush() to SkiaGL's GLContext on memory-pressure
Review of attachment 8481780 [details] [diff] [review]:
-----------------------------------------------------------------
Looks fine then, but please add a comment about the flush call to say that GrContext::flush() doesn't call glFlush.
Attachment #8481780 -
Flags: review?(gwright) → review+
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → sotaro.ikeda.g
Assignee | ||
Comment 8•10 years ago
|
||
Apply the comment.
Attachment #8481780 -
Attachment is obsolete: true
Attachment #8482944 -
Flags: review+
Assignee | ||
Comment 9•10 years ago
|
||
Updated•10 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Assignee | ||
Comment 10•10 years ago
|
||
Comment 11•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 12•10 years ago
|
||
Note that due to recent policy changes, all B2G uplifts needs approval now regardless of blocking status. Please request aurora and b2g32 approval on this patch when you get a chance. Sorry for the inconvenience.
status-b2g-v2.0:
--- → affected
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → fixed
status-firefox33:
--- → wontfix
status-firefox34:
--- → affected
status-firefox35:
--- → fixed
Updated•10 years ago
|
Flags: needinfo?(sotaro.ikeda.g)
Assignee | ||
Comment 13•10 years ago
|
||
Comment on attachment 8482944 [details] [diff] [review]
patch - Call glFlush() to SkiaGL's GLContext on memory-pressure r=gw280
NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.
[Approval Request Comment]
Bug caused by (feature/regressing bug #): None.
User impact if declined: An application might be killed more than the fix is applied.
Testing completed: Tested locally on flame device.
Risk to taking this patch (and alternatives if risky): Low.
String or UUID changes made by this patch:
Attachment #8482944 -
Flags: approval-mozilla-b2g32?
Attachment #8482944 -
Flags: approval-mozilla-aurora?
Flags: needinfo?(sotaro.ikeda.g)
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #13)
>
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): None.
> User impact if declined: An application might be killed more than the fix is
> applied.
An application might be killed more than the fix is applied because of low memory.
Updated•10 years ago
|
Attachment #8482944 -
Flags: approval-mozilla-b2g32?
Attachment #8482944 -
Flags: approval-mozilla-b2g32+
Attachment #8482944 -
Flags: approval-mozilla-aurora?
Attachment #8482944 -
Flags: approval-mozilla-aurora+
Comment 15•10 years ago
|
||
Updated•10 years ago
|
status-b2g-v2.0M:
--- → fixed
Comment 16•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•