Last Comment Bug 743314 - Force CanUploadSubtextures on Maemo6 harmattan
: Force CanUploadSubtextures on Maemo6 harmattan
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: Trunk
: ARM Maemo
: -- normal (vote)
: mozilla15
Assigned To: Oleg Romashin (:romaxa)
:
Mentors:
Depends on:
Blocks: 727688
  Show dependency treegraph
 
Reported: 2012-04-06 11:59 PDT by Oleg Romashin (:romaxa)
Modified: 2012-04-29 15:59 PDT (History)
5 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Disable workaround driver bugs on maemo (572 bytes, patch)
2012-04-11 17:55 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
Quick fix (don't really like this) (891 bytes, patch)
2012-04-12 18:15 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Splinter Review
Check for KHR lock Surface extension (2.14 KB, patch)
2012-04-12 18:24 PDT, Oleg Romashin (:romaxa)
chrislord.net: review-
Details | Diff | Splinter Review
Check for lock Surface feature (2.13 KB, patch)
2012-04-27 13:20 PDT, Oleg Romashin (:romaxa)
chrislord.net: review+
Details | Diff | Splinter Review

Description Oleg Romashin (:romaxa) 2012-04-06 11:59:59 PDT
I've been testing latest m-c where we enabled texture upload checks and small tiles for all platforms, and I've found that scrolling on simple page on N9 become more jerkiness...

Problem is that making small tiles increased amount of TextureLock/Unlock operations, and that is making related Sync/Flushes
Comment 1 Oleg Romashin (:romaxa) 2012-04-11 17:55:18 PDT
Created attachment 614240 [details] [diff] [review]
Disable workaround driver bugs on maemo

tested gfx.work-around-driver-bugs preference and with = TRUE (default) i see on scaled rendering:
Compositor: Layers update took 28 ms (blocking gecko).
Compositor: Layers update took 378 ms (blocking gecko).
Compositor: Layers update took 391 ms (blocking gecko).
Compositor: Layers update took 396 ms (blocking gecko).

With FALSE value
Compositor: Layers update took 18 ms (blocking gecko).
Compositor: Layers update took 32 ms (blocking gecko).
Compositor: Layers update took 111 ms (blocking gecko).

So suggesting to set that pref -> FALSE on maemo, because it make things slower.
Comment 2 Benoit Girard (:BenWa) 2012-04-11 18:13:14 PDT
gfx.work-around-driver-bugs is going to be used for many things in the future. Perhaps it would be best to find what in particular is slower, such as CanUploadSubtextures and disable individual work around that aren't needed at compile time.
Comment 3 Oleg Romashin (:romaxa) 2012-04-11 18:14:51 PDT
Comment on attachment 614240 [details] [diff] [review]
Disable workaround driver bugs on maemo

ok, I see, will do that
Comment 4 Oleg Romashin (:romaxa) 2012-04-12 18:15:46 PDT
Created attachment 614642 [details] [diff] [review]
Quick fix (don't really like this)

I see two options here:
one is simply  force subtextures for Maemo environment... but that would be hard to use if I build non-maemo build on N9 hardware...

Another option is to check EGL provider if that has KHRLockSurface (which allow us to upload sub images and even render directly into texture memory data) and return true in that case.
Comment 5 Oleg Romashin (:romaxa) 2012-04-12 18:24:20 PDT
Created attachment 614644 [details] [diff] [review]
Check for KHR lock Surface extension
Comment 6 Oleg Romashin (:romaxa) 2012-04-25 01:15:53 PDT
Also this causing webgl breakage, because on N9 we use backingSurface and that seems not very friendly with small tiles.
Comment 7 Chris Lord [:cwiiis] 2012-04-27 13:10:04 PDT
Comment on attachment 614644 [details] [diff] [review]
Check for KHR lock Surface extension

Review of attachment 614644 [details] [diff] [review]:
-----------------------------------------------------------------

An r- from me, but only due to naming issues.

::: gfx/gl/GLContext.cpp
@@ +640,5 @@
>  {
>      if (!mWorkAroundDriverBugs)
>          return true;
>  
> +    if (CanRenderToTexture())

This doesn't make sense, logically - Rendering to a texture doesn't necessarily help doing a sub-texture upload... Perhaps this function should have been called HasFastTextureUpload, or something along those lines?

::: gfx/gl/GLContextProviderEGL.cpp
@@ +596,5 @@
>  
>          return h;
>      }
>  
> +    virtual bool CanRenderToTexture() {

KHRLockSurface isn't really render-to-texture, it's mmap-texture-memory... This needs a better name.
Comment 8 Oleg Romashin (:romaxa) 2012-04-27 13:20:13 PDT
Created attachment 619159 [details] [diff] [review]
Check for  lock Surface feature
Comment 9 Chris Lord [:cwiiis] 2012-04-28 03:02:15 PDT
Comment on attachment 619159 [details] [diff] [review]
Check for  lock Surface feature

Review of attachment 619159 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with an added comment explaining why you would want to allow sub-texture upload when HasLockSurface is true.

::: gfx/gl/GLContext.cpp
@@ +640,5 @@
>  {
>      if (!mWorkAroundDriverBugs)
>          return true;
>  
> +    if (HasLockSurface())

Add a comment here as to why we're returning true when HasLockSurface returns true.
Comment 11 Ryan VanderMeulen [:RyanVM] 2012-04-29 15:59:56 PDT
http://hg.mozilla.org/mozilla-central/rev/339e0c1de0e1

Note You need to log in before you can comment on or make changes to this bug.