Use Small tiles should not be android only.

RESOLVED FIXED in mozilla14

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: romaxa, Assigned: romaxa)

Tracking

Trunk
mozilla14
ARM
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
Created attachment 608928 [details] [diff] [review]
More generic sub texture upload detection

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 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

6 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 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

6 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

6 years ago
Created attachment 609019 [details] [diff] [review]
Use small tiles not only on android
Attachment #608928 - Attachment is obsolete: true
Attachment #609019 - Flags: review?
(Assignee)

Updated

6 years ago
Attachment #609019 - Flags: review? → review?(bgirard)
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

6 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c14032c09e1
Assignee: nobody → romaxa
Flags: in-testsuite?
Keywords: checkin-needed
Target Milestone: --- → mozilla14

Comment 8

6 years ago
https://hg.mozilla.org/mozilla-central/rev/2c14032c09e1
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.