Bug 58731 - [Texmap] [Qt] Do not reset a texture if it comes from imageToTexture pool.
Summary: [Texmap] [Qt] Do not reset a texture if it comes from imageToTexture pool.
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Qt, QtTriaged
Depends on: 47070 58903
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-16 10:54 PDT by Young Han Lee
Modified: 2012-02-14 04:35 PST (History)
5 users (show)

See Also:


Attachments
Patch (1.91 KB, patch)
2011-04-16 11:06 PDT, Young Han Lee
eric: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Young Han Lee 2011-04-16 10:54:32 PDT
imageToTexture is a pool of textures to share it between BitmapTextureGLs.

The textures in the pool are already initialized and filled with image data, so they doesn't have to be initialized again when it comes from the pool.

The current implementation always reset the texture whether it is found in the pool or new. This causes a problem that openGL draws a texture, which has garbage data, to screen.
Comment 1 Young Han Lee 2011-04-16 11:06:08 PDT
Created attachment 89926 [details]
Patch
Comment 2 Noam Rosenthal 2011-04-16 11:15:04 PDT
LGTM
Comment 3 Benjamin Poulain 2011-04-17 05:16:28 PDT
Comment on attachment 89926 [details]
Patch

Any reason not to make a pixel test for the bug?
Comment 4 Young Han Lee 2011-04-17 19:36:27 PDT
I'm not sure it's possible to test animation features like this using a pixel test.
If so, please let me know any hint.
Comment 5 Noam Rosenthal 2011-04-17 20:42:58 PDT
(In reply to comment #3)
> (From update of attachment 89926 [details])
> Any reason not to make a pixel test for the bug?

We still don't have a way to pixel-test the opengl backend.
If we had, the tests under LayoutTests/compositing should be sufficient for these optimizations.
Comment 6 Eric Seidel (no email) 2011-04-18 09:26:49 PDT
Comment on attachment 89926 [details]
Patch

How do we test this?
Comment 7 Kenneth Russell 2011-04-18 13:16:33 PDT
In Chromium glReadPixels is used in order to run pixel tests for the accelerated compositor.
Comment 8 Young Han Lee 2011-04-18 18:49:39 PDT
(In reply to comment #7)
> In Chromium glReadPixels is used in order to run pixel tests for the accelerated compositor.

Could you refer me to the Chrominum's pixel test cases or DRT code for those tests which is using glReadPixels?
I failed to find how Chromium does to test accelerated compositor in WebKit source.
Comment 9 Kenneth Russell 2011-04-18 19:20:32 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > In Chromium glReadPixels is used in order to run pixel tests for the accelerated compositor.
> 
> Could you refer me to the Chrominum's pixel test cases or DRT code for those tests which is using glReadPixels?
> I failed to find how Chromium does to test accelerated compositor in WebKit source.

Chromium's test expectations for tests involving the compositor are checked in under LayoutTests/platform/chromium-gpu/compositing/.

There are several places in Chromium's code where the behavior is switched between the full browser and DRT. One example is using the real OpenGL on the system vs. the Mesa software renderer. However, the key place where the readback is done for DRT is in WebViewImpl::paint, Source/WebKit/chromium/src/WebViewImpl.cpp. When running in DRT, the passed-in WebCanvas is non-NULL, and the readback is done.
Comment 10 Young Han Lee 2011-04-19 09:08:04 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > In Chromium glReadPixels is used in order to run pixel tests for the accelerated compositor.
> > 
> > Could you refer me to the Chrominum's pixel test cases or DRT code for those tests which is using glReadPixels?
> > I failed to find how Chromium does to test accelerated compositor in WebKit source.
> 
> Chromium's test expectations for tests involving the compositor are checked in under LayoutTests/platform/chromium-gpu/compositing/.
> 
> There are several places in Chromium's code where the behavior is switched between the full browser and DRT. One example is using the real OpenGL on the system vs. the Mesa software renderer. However, the key place where the readback is done for DRT is in WebViewImpl::paint, Source/WebKit/chromium/src/WebViewImpl.cpp. When running in DRT, the passed-in WebCanvas is non-NULL, and the readback is done.

Thanks for the detailed reply.

It seems that Qt's WebKit and DRT also have to be modified in the same way for a pixel-test.

I will try to patch this in the near future.
Comment 11 Noam Rosenthal 2012-02-14 04:35:43 PST
Code has completely changed, this no longer applies.