Closed
Bug 738865
Opened 12 years ago
Closed 12 years ago
Use Small tiles should not be android only.
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: romaxa, Assigned: romaxa)
Details
Attachments
(1 file, 1 obsolete file)
1.45 KB,
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
I was playing with b2g on beagle board, and tried to figure out why it is fast on S Galaxy II and slow in my case... by checking vendors/renderers tricks I found that using small tiles and avoid subTexture upload makes rendering as on beagleboard as fast as on S Galaxy II... because powervr SGX 530 there has similar problem. I'm wondering can we make that check more generic? with this fix, I have FPS increase for http://bubblemark.com/dhtml.htm from 10FPS->45FPS
Attachment #608928 -
Flags: review?(pwalton)
Comment 1•12 years ago
|
||
Comment on attachment 608928 [details] [diff] [review] More generic sub texture upload detection Looks fine to me, but Chris should review this.
Attachment #608928 -
Flags: review?(pwalton)
Attachment #608928 -
Flags: review?(chrislord.net)
Attachment #608928 -
Flags: feedback+
Comment 2•12 years ago
|
||
Comment on attachment 608928 [details] [diff] [review] More generic sub texture upload detection Review of attachment 608928 [details] [diff] [review]: ----------------------------------------------------------------- r+ with the function renamed, but I think BenWa should review this really (sorry for the ping-pong). ::: gfx/gl/GLContext.cpp @@ +618,5 @@ > return false; > > // Don't use small tiles otherwise. (If we implement incremental texture upload, > // then we will want to revisit this.) > return false; whoops, we have implemented incremental texture upload, we should probably review this... ::: gfx/gl/GLContext.h @@ +712,5 @@ > > bool CanUploadSubTextures(); > bool CanUploadNonPowerOfTwo(); > bool WantsSmallTiles(); > + virtual bool SupportRenderToTexture() { return true; } This doesn't return whether render-to-texture is available, but whether fast texture upload is available. It should be renamed to reflect what it does, and it should return false by default. Perhaps HasFastDirectUpdate() instead?
Attachment #608928 -
Flags: review?(chrislord.net)
Attachment #608928 -
Flags: review?(bgirard)
Attachment #608928 -
Flags: review+
Comment 3•12 years ago
|
||
Comment on attachment 608928 [details] [diff] [review] More generic sub texture upload detection Review of attachment 608928 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/gl/GLContext.cpp @@ +578,5 @@ > > // On PowerVR glTexSubImage does a readback, so it will be slower > // than just doing a glTexImage2D() directly. i.e. 26ms vs 10ms > if (Renderer() == RendererSGX540 || Renderer() == RendererSGX530) > + return SupportRenderToTexture(); I don't want to make a change like until we some performance number, and ideally a small benchmark hack patch and/or some details on how it measure. We've made several assumptions in the pass about performance and checking them after a change is made is very tedious. I'd be happy to take this patch without this change until we get these numbers.
Attachment #608928 -
Flags: review?(bgirard) → review-
Assignee | ||
Comment 4•12 years ago
|
||
perf numbers are in first comment, also there are perf numbers in right above that check:
>// than just doing a glTexImage2D() directly. i.e. 26ms vs 10ms
Direct texture rendering with RendererSGX530 supported only on N9... all other devices having same problem as android
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #608928 -
Attachment is obsolete: true
Attachment #609019 -
Flags: review?
Assignee | ||
Updated•12 years ago
|
Attachment #609019 -
Flags: review? → review?(bgirard)
Comment 6•12 years ago
|
||
Comment on attachment 609019 [details] [diff] [review] Use small tiles not only on android As discussed on IRC I was asking for perf numbers for KHRLock, feel free to submit a patch for that if we have evidence that it is faster.
Attachment #609019 -
Flags: review?(bgirard) → review+
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 7•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c14032c09e1
Assignee: nobody → romaxa
Flags: in-testsuite?
Keywords: checkin-needed
Target Milestone: --- → mozilla14
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2c14032c09e1
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•12 years ago
|
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•