Bug 219256 - GraphicsContextGL should have robust multivalue setters
Summary: GraphicsContextGL should have robust multivalue setters
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on:
Blocks: webglgpup 219320
  Show dependency tree / graph
 
Reported: 2020-11-23 05:17 PST by Kimmo Kinnunen
Modified: 2020-11-29 23:59 PST (History)
10 users (show)

See Also:

Attachments
Patch (169.92 KB, patch)
2020-11-24 05:16 PST, Kimmo Kinnunen 		no flags Details | Formatted Diff | Diff
Patch (173.39 KB, patch)
2020-11-24 05:27 PST, Kimmo Kinnunen 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (174.26 KB, patch)
2020-11-24 05:40 PST, Kimmo Kinnunen 		no flags Details | Formatted Diff | Diff
Patch (175.37 KB, patch)
2020-11-24 05:48 PST, Kimmo Kinnunen 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (176.23 KB, patch)
2020-11-24 06:50 PST, Kimmo Kinnunen 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (187.65 KB, patch)
2020-11-24 07:09 PST, Kimmo Kinnunen 		no flags Details | Formatted Diff | Diff
Patch (187.55 KB, patch)
2020-11-24 07:17 PST, Kimmo Kinnunen 		no flags Details | Formatted Diff | Diff
Patch (196.56 KB, patch)
2020-11-26 05:16 PST, Kimmo Kinnunen 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (198.93 KB, patch)
2020-11-26 05:26 PST, Kimmo Kinnunen 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (198.94 KB, patch)
2020-11-26 05:38 PST, Kimmo Kinnunen 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (198.96 KB, patch)
2020-11-26 05:43 PST, Kimmo Kinnunen 		no flags Details | Formatted Diff | Diff
Patch (199.06 KB, patch)
2020-11-26 05:50 PST, Kimmo Kinnunen 		no flags Details | Formatted Diff | Diff
Show Obsolete (11) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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