Bug 219256

Summary: GraphicsContextGL should have robust multivalue setters
Product: WebKit Reporter: Kimmo Kinnunen <kkinnunen>
Component: WebGLAssignee: Kimmo Kinnunen <kkinnunen>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, changseok, dino, esprehn+autocc, ews-watchlist, graouts, gyuyoung.kim, kondapallykalyan, pnormand, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=219340
Bug Depends on:    
Bug Blocks: 217211, 219320    
Attachments:
Description Flags
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch none

Description Kimmo Kinnunen 2020-11-23 05:17:37 PST
GraphicsContextGL should have robust multivalue setters

Instead of passing `size, ptr`
we should pass `GCGLSpan(ptr, size)`
in all functions taking buffer parameters.
Comment 1 Kimmo Kinnunen 2020-11-24 05:16:29 PST
Created attachment 414831 [details]
Patch
Comment 2 Kimmo Kinnunen 2020-11-24 05:27:27 PST
Created attachment 414832 [details]
Patch
Comment 3 Kimmo Kinnunen 2020-11-24 05:40:06 PST
Created attachment 414833 [details]
Patch
Comment 4 Kimmo Kinnunen 2020-11-24 05:48:31 PST
Created attachment 414834 [details]
Patch
Comment 5 Kimmo Kinnunen 2020-11-24 06:50:24 PST
Created attachment 414837 [details]
Patch
Comment 6 Kimmo Kinnunen 2020-11-24 07:09:13 PST
Created attachment 414839 [details]
Patch
Comment 7 Kimmo Kinnunen 2020-11-24 07:17:36 PST
Created attachment 414840 [details]
Patch
Comment 8 Dean Jackson 2020-11-25 01:29:40 PST
Comment on attachment 414840 [details]
Patch

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

Very nice.

> Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1132
> -    m_context->getExtensions().texImage2DRobustANGLE(target, level, internalformat, width, height, border, format, type, 0, reinterpret_cast<void*>(offset));
> +    m_context->texImage2D(target, level, internalformat, width, height, border, format, type, offset);

Was this intentional? Does texImage2D call the robust ANGLE form itself?
Comment 9 Kimmo Kinnunen 2020-11-26 05:16:58 PST
Created attachment 414895 [details]
Patch
Comment 10 Kimmo Kinnunen 2020-11-26 05:21:49 PST
(In reply to Dean Jackson from comment #8)
> > Source/WebCore/html/canvas/WebGL2RenderingContext.cpp:1132
> > -    m_context->getExtensions().texImage2DRobustANGLE(target, level, internalformat, width, height, border, format, type, 0, reinterpret_cast<void*>(offset));
> > +    m_context->texImage2D(target, level, internalformat, width, height, border, format, type, offset);
> 
> Was this intentional? Does texImage2D call the robust ANGLE form itself?

Yes. Sorry for not describing it in the ChangeLog and not carrying the change through its logical conclusion.

I uploaded a new patch, which sohuld clarify this in the ChangeLog and in the code. In the code, I moved all the texImage functions from the ExtensionsGL to GraphicsContextGL.
Comment 11 Kimmo Kinnunen 2020-11-26 05:26:00 PST
Created attachment 414896 [details]
Patch
Comment 12 Kimmo Kinnunen 2020-11-26 05:38:47 PST
Created attachment 414897 [details]
Patch
Comment 13 Kimmo Kinnunen 2020-11-26 05:43:53 PST
Created attachment 414898 [details]
Patch
Comment 14 Kimmo Kinnunen 2020-11-26 05:50:28 PST
Created attachment 414899 [details]
Patch
Comment 15 EWS 2020-11-27 01:52:03 PST
Committed r270185: <https://trac.webkit.org/changeset/270185>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 414899 [details].
Comment 16 Radar WebKit Bug Importer 2020-11-27 01:53:16 PST
<rdar://problem/71761224>
Comment 17 Philippe Normand 2020-11-27 03:17:02 PST
Comment on attachment 414899 [details]
Patch

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

> Source/WebCore/platform/graphics/GraphicsTypesGL.h:72
> +    GCGLSpan(const GCGLSpan<U, Extent>& other, std::enable_if_t<std::is_convertible_v<U(*)[], T(*)[]>, nullptr_t> = nullptr)

Source/WebCore/platform/graphics/GraphicsTypesGL.h:72:104: error: unknown type name 'nullptr_t'; did you mean 'std::nullptr_t'?
    GCGLSpan(const GCGLSpan<U, Extent>& other, std::enable_if_t<std::is_convertible_v<U(*)[], T(*)[]>, nullptr_t> = nullptr)
                                                                                                       ^~~~~~~~~
                                                                                                       std::nullptr_t
/usr/bin/../lib/gcc/x86_64-unknown-linux-gnu/10.2.0/../../../../include/c++/10.2.0/x86_64-unknown-linux-gnu/bits/c++config.h:264:29: note: 'std::nullptr_t' declared here
  typedef decltype(nullptr) nullptr_t;
                            ^
Comment 18 Philippe Normand 2020-11-27 03:22:06 PST
r270189
Comment 19 Kimmo Kinnunen 2020-11-29 23:59:23 PST
Thanks Philippe