Bug 244526

Summary: TextureMapper: Attach a depth buffer for BitmapTextureGL for 3D transform
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: PlatformAssignee: Fujii Hironori <Hironori.Fujii>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, don.olmstead, ews-watchlist, kondapallykalyan, luiz, magomez, webkit-bug-importer, zdobersek
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=90078
https://bugs.webkit.org/show_bug.cgi?id=254262
Bug Depends on:    
Bug Blocks: 241699    
Attachments:
Description Flags
Patch
none
Patch
none
[fast-cq] Patch for landing none

Description Fujii Hironori 2022-08-29 19:38:06 PDT 
TextureMapper: Attach a depth buffer for BitmapTextureGL for 3D transform

The current implementation is using only the depth buffer of the default framebuffer.
To support nesting 3D rendering context (bug#241699), I'm going to attach a depth buffer for BitmapTextureGL.
Comment 1 Fujii Hironori 2022-08-29 19:44:48 PDT 
Created attachment 461998 [details]
Patch
Comment 2 Fujii Hironori 2022-08-30 13:03:57 PDT 
Comment on attachment 461998 [details]
Patch

GTK EWS reported test failures.
depth bits isn't enough. Using GL_DEPTH_COMPONENT24 passes the tests, But, this isn't available for ES2.0.
Comment 3 Fujii Hironori 2022-08-31 21:21:04 PDT 
I found out another problem for WinCairo.
It seems that ANGLE can't attach both a depth buffer and a stencil buffer to a fbo.
WinCairo has to use a single depth/stencil buffer of GL_DEPTH24_STENCIL8_OES.
Linux doesn't seem to have such problem.
Comment 4 Radar WebKit Bug Importer 2022-09-05 19:39:15 PDT 
<rdar://problem/99581351>
Comment 5 Fujii Hironori 2022-09-29 14:38:43 PDT 
Created attachment 462718 [details]
Patch
Comment 6 Don Olmstead 2022-09-29 15:02:05 PDT 
Comment on attachment 462718 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=462718&action=review

r=me with nits

> Source/WebCore/platform/graphics/texmap/BitmapTextureGL.h:45
> +#if OS(WINDOWS)
> +#define USE_TEXMAP_DEPTH_STENCIL_BUFFER 1
> +#else
> +#define USE_TEXMAP_DEPTH_STENCIL_BUFFER 0
> +#endif

From your commit message it seems like this might be better to be `#if USE(ANGLE)`?

Also I feel like this should be in a WTF header not just hanging out here.
Comment 7 Fujii Hironori 2022-09-29 17:00:58 PDT 
Comment on attachment 462718 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=462718&action=review

Thank you very much for the review.

>> Source/WebCore/platform/graphics/texmap/BitmapTextureGL.h:45
>> +#endif
> 
> From your commit message it seems like this might be better to be `#if USE(ANGLE)`?
> 
> Also I feel like this should be in a WTF header not just hanging out here.

GTK and WPE also enable USE_ANGLE. USE_ANGLE is nonsense anymore. We should remove it.
I don't want to recompile whole WebKit just by changing TextureMapper configs.
Not all USE_ macro definitions are in WTF. I think it's OK to be here.
Comment 8 Fujii Hironori 2022-09-29 21:45:12 PDT 
Created attachment 462723 [details]
[fast-cq] Patch for landing
Comment 9 EWS 2022-09-29 21:47:46 PDT 
Committed 255021@main (0793eb5a3182): <https://commits.webkit.org/255021@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 462723 [details].