[Webkit-unassigned] [Bug 113493] Enhance the restrictions on going through the faster hardware path for uploading Canvas2D to WebGL

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=113493





--- Comment #6 from Jun Jiang <jun.a.jiang at intel.com>  2013-03-29 07:09:35 PST ---
(From update of attachment 195510)
View in context: https://bugs.webkit.org/attachment.cgi?id=195510&action=review

>> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:3877
>> +        && (texture->getType(target, level) == GraphicsContext3D::UNSIGNED_BYTE || !texture->isValid(target, level))
> 
> Actually I don't understand why we care about target texture's previous type?  Looking at https://code.google.com/p/chromium/codesearch#chromium/src/gpu/GLES2/extensions/CHROMIUM/CHROMIUM_copy_texture.txt
> 
> I don't see why previous type is relevant.  Probably you can just remove this condition.

Hi, Zhenyao. Thanks for your comments.
There are two kinds of check on "type" here. One is for the target type, shown as "type == GraphicsContext3D::UNSIGNED_BYTE"; it was used as our white-list together with restrictions on format(RGB/RGBA) since some format/types are not supported in gpu::gles2::CopyTextureCHROMIUMResourceManager::CopyTextureCHROMIUM(). The other is what you mentioned on the check for previous type; It was added here because in gpu::gles2::GLES2DecoderImpl::DoCopyTextureCHROMIUM(), the implementation is like this: 1) if the destination texture is defined, the type is determined by its previous value even the destination texture is redefined; 2). if the destination texture is not defined, it would be defined but the type is determined by that of its source texture.
That's why the check goes like this. I am not quite sure about several things at present, for example, what other types exactly will be supported besides UNSIGNED_INT in the future, which type will be the default type when the destination texture is defined or re-defined when multiple types are supported in DoCopyTextureCHROMIUM(), etc. It seems some discussion is already being happened at chromium side(see https://code.google.com/p/chromium/issues/detail?id=153925). Before that happens, we can make the white-list works better.

>> Source/WebCore/html/canvas/WebGLRenderingContext.cpp:3878
>> +        && !level) {
> 
> Instead of limiting to level == 0 here, why don't we add an extra parameter level to copyToPlatformTexture()?

Yes, we can pass a level parameter to copyToPlatformTexture() to avoid hard-coding the magic number 0 as parameter. But the limitation on "level == 0" may be still needed. In fact, the limitation on that "level == 0" is because we do the texture -> (FBO-texture) copy which uses glFrameBufferTexture2D(..., level) in DoCopyTextureCHROMIUM() where the level must be 0.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.