Re: [webkit-dev] Compile error in WebGL2RenderingContext.cpp

2019-01-17 Thread Michael Catanzaro
On Thu, Jan 17, 2019 at 1:13 PM, Maciej Stachowiak  
wrote:

std::array is fixed size.


Er, yeah, oops. Size must be known at compile time, so it can't be used 
to replace a VLA. Vector it is


___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Compile error in WebGL2RenderingContext.cpp

2019-01-17 Thread Maciej Stachowiak


> On Jan 17, 2019, at 10:15 AM, Michael Catanzaro  wrote:
> 
> 
> 
> On Thu, Jan 17, 2019 at 11:12 AM, Darin Adler  wrote:
>> Vector’s inline capacity feature was originally created as an alternative to 
>> variable length arrays for most of the purposes people would want to put 
>> them.
> 
> Any advantages of this over std::array (which is widely-used in WebKit)?

std::array is fixed size. So you can’t do Darin’s described trick of using the 
stack for small enough cases but falling back to the heap for large cases, for 
a  buffer whose size is not known at compile time.

std::array seems fine to use for cases that are known to be fixed size at 
compile time. I would imagine it saves space by not having to separately track 
size and capacity. I don’t know if std::array holds its values inline or in an 
external heap buffer always. If it always uses a heap buffer, then it’s 
probably  worse than a normal C array for strictly local buffers. It could 
still be ok for fixed-size arrays stored in objects.

Regards,
Maciej
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Compile error in WebGL2RenderingContext.cpp

2019-01-17 Thread Michael Catanzaro



On Thu, Jan 17, 2019 at 11:12 AM, Darin Adler  wrote:
Vector’s inline capacity feature was originally created as an 
alternative to variable length arrays for most of the purposes people 
would want to put them.


Any advantages of this over std::array (which is widely-used in WebKit)?

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Compile error in WebGL2RenderingContext.cpp

2019-01-17 Thread Darin Adler
Vector’s inline capacity feature was originally created as an alternative to 
variable length arrays for most of the purposes people would want to put them.

Imagine, for example, that you need a variable length buffer of characters that 
is almost always going to be less then 32 bytes. You write this:

 Vector buffer;

You can then grow the buffer to whatever size you need. If it’s less than 32 
bytes then it uses space on the stack, and if more than 32 bytes it uses space 
on the heap.

The reason this is better than variable length arrays is that stack size often 
has a inflexible total limit, so it’s not OK in general to use an arbitrary 
amount of stack. The vector inline capacity allows us to easily use up to a 
fixed amount of stack, but then still correctly handle unusually large requests 
by spilling go the heap.

For that WebGL2 code the idiom would be something like this:

Vector parameters;
parameters.grow(numValues);

Not sure we have good Vector inline capacity documentation, but that’s a basic 
primer.

— Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Compile error in WebGL2RenderingContext.cpp

2019-01-17 Thread Michael Catanzaro
On Thu, Jan 17, 2019 at 2:27 AM, Salisbury, Mark 
 wrote:

Thanks Tim!



I didn’t know about C++ variable length arrays.  That makes perfect 
sense now.




Looks like people have been requesting Visual Studio add support for 
VLAs; there are no indications they will be supported.


We shouldn't be using VLAs in WebKit. It's not standard C++, and even 
if it was, it's not safe. The code should use std::array or perhaps 
WTF::Vector.


Michael

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Compile error in WebGL2RenderingContext.cpp

2019-01-17 Thread Salisbury, Mark
Thanks Tim!

I didn’t know about C++ variable length arrays.  That makes perfect sense now.

Looks like people have been requesting Visual Studio add support for VLAs; 
there are no indications they will be supported.

From: thor...@apple.com 
Sent: Thursday, January 17, 2019 3:50 PM
To: Salisbury, Mark 
Cc: webkit-dev@lists.webkit.org
Subject: Re: [webkit-dev] Compile error in WebGL2RenderingContext.cpp




On Jan 16, 2019, at 20:56, Salisbury, Mark 
mailto:mark.salisb...@hp.com>> wrote:

Hello,

Hello!


I’m working on rebasing our downstream webkit port; I just pulled down recent 
webkit changes (since July ’18) and I hit a piece of code that’s failing to 
compile in Visual Studio 2017.  When I look at the code however, I’m not sure 
how it compiles on any compiler, or if it compiles, how it’s safe.

This is from Source/WebCore/html/canvas/WebGL2RenderingContext.cpp -

…
int numValues = 0;
#if USE(OPENGL_ES)
m_context->getInternalformativ(target, internalformat, 
GraphicsContext3D::NUM_SAMPLE_COUNTS, 1, );

GC3Dint params[numValues];
m_context->getInternalformativ(target, internalformat, pname, numValues, 
params);
#else
// On desktop OpenGL 4.1 or below we must emulate glGetInternalformativ.

Visual Studio doesn’t like it because it won’t instantiate an array without a 
const size.  On the other hand, even if it compiled, passing an array with 0 
size doesn’t make a lot of sense, to receive parameters back, which appears to 
be the intent of the code.

Why do you think it would be 0?

numValues is passed as an out arg in the first getInternalformativ call, and is 
presumably filled with the value for the key NUM_SAMPLE_COUNTS.

That said, this depends on a C99 feature that is maybe only available as a 
compiler extension in C++ and it sounds like isn't supported by the VS compiler 
("variable length arrays"). So I'm guessing nobody has built it with VS yet.



The file is associated with WebGL2, but it’s added to the build in the 
ENABLE_WEBGL block in WebCore/CMakeLists.txt.
WEBGL2 is off by default (per WebKitFeatures.cmake) so I assume the code could 
not be invoked without enabling WEBGL 2.

The contents of this file are guarded with #if ENABLE(WEBGL2); you should 
probably figure out why that's turned on if you don't expect it.


Am I looking at unfinished code that magically compiles with gcc/llvm/etc. (non 
Visual Studio)?

Thanks,
Mark




___
webkit-dev mailing list
webkit-dev@lists.webkit.org<mailto:webkit-dev@lists.webkit.org>
https://lists.webkit.org/mailman/listinfo/webkit-dev<https://lists.webkit.org/mailman/listinfo/webkit-dev>

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev