[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Public WebGL] Proposal: Restrict PACK_/UNPACK_ params to sane subrects



Olli's proposal sounds good to me.

Mo

On Mon, Jul 11, 2016 at 1:39 AM, Olli Etuaho <oetuaho@nvidia.com> wrote:

I’m in favor of forbidding overlapping rows and overlapping images, I doubt that developers would be harmed by this restriction in any way.

 

I’m not that sure of restricting SKIP_PIXELS to be below row length – SKIP_PIXELS doesn’t seem that difficult to implement or use, and I’m not aware of any driver bugs affecting cases where SKIP_PIXELS is large.

 

-Olli

 

From: owners-public_webgl@khronos.org [mailto:owners-public_webgl@khronos.org] On Behalf Of Zhenyao Mo
Sent: sunnuntaina 10. heinäkuuta 2016 22.28
To: Jeff Gilbert <jgilbert@mozilla.com>
Cc: webgl, public <public_webgl@khronos.org>
Subject: Re: [Public WebGL] Proposal: Restrict PACK_/UNPACK_ params to sane subrects

 

Looking at Chrome implementation, it does seem the driver bugs in this area are mostly in handing the cases you propose to forbid. I am on the fence.

 

On Sun, Jul 10, 2016 at 11:47 AM, Zhenyao Mo <zmo@chromium.org> wrote:

I see your points. On top of my head, I don't see a good use case as an counter argument.

 

However, I feel these arguments should be made in defining ES3 spec, whereas WebGL 2 spec should mostly follow ES3 spec unless there is security concern, major perf concern, or something impossible to implement. I would be very careful in modifying the spec just because we see no apparent use case.  We made that mistake before with IMPLEMENTATION_READ_FORMAT/TYPE.

 

On Fri, Jul 8, 2016 at 9:57 PM, Jeff Gilbert <jgilbert@mozilla.com> wrote:

Mathematically, these are equivalent.
See GLES 3.0.4 p117 Figure 3.6, which is conceptually similar to my description.

Unfettered behavior here, however, not make sense in the context of
the rest of the API.

There are two relevant cases:
* Overlapping rows
* Non-trivial subrects

Overlapping rows:
width>ROW_LENGTH
(The same applies to height>IMAGE_HEIGHT)
Overlapping rows are not tightly defined. As such, well-behaved apps
cannot expect the results of an overlapping row read to be consistent
across implementations. Since it's trivial to change the params into a
non-overlapped row configuration, with no loss of reliable utility, I
propose we restrict this behavior. 


Nontrivial subrects:
SKIP_PIXELS+width>(ROW_LENGTH ? ROW_LENGTH : width)
(The same applies for SKIP_ROWS/IMAGE_HEIGHT)
If the goal is simply to offset into the buffer, there are other
entrypoints/API aspects which allow this.
However, there are weird results from using something like
SKIP_PIXEL=10000: Alignment (for the purposes of row striding) is
still based on effective row length alone. (effective row length =
ROW_LENGTH ? ROW_LENGTH : width) Thus `baseOffset + skipBytes +
N*rowStride` will not necessarily be aligned to `baseOffset +
M*ALIGNMENT`, subverting the ALIGNMENT param. (and generally being
suprising!)
Roughly, if `SKIP_PIXELS > ROW_LENGTH-width`, you can just offset the
base pointer via another part of the API, then make a call with
`SKIP_PIXELS <= ROW_LENGTH-width`.
Since there are other ways to handle arbitrary byte skipping, and
eliminating nontrivial subrects is an easy way to ensure expected
behavior, I propose we restrict this behavior.


On Fri, Jul 8, 2016 at 5:07 PM, Zhenyao Mo <zmo@chromium.org> wrote:
> My understanding of these parameters is slightly different.
>
> SKIP_PIXELS is only applied once, not per row.  Say, I can just skip 10000
> pixels.
>
> Again, SKIP_ROWS is also only applied once, not per image.
>
>
>
> On Fri, Jul 8, 2016 at 11:57 AM, Jeff Gilbert <jgilbert@mozilla.com> wrote:
>>
>>
>> Specifically, the following would generate INVALID_OPERATION:
>> * SKIP_PIXELS + width > ROW_LENGTH
>> * SKIP_ROWS + height > IMAGE_HEIGHT
>>
>> This will:
>> * Make things cleaner for both implementers and devs
>> * Remove a source of implementation behavior differences
>> * Prevent a source of bugs for developers (setting SKIP_PIXELS without
>> ROW_LENGTH)
>>
>> If we find a compelling use-case for relaxing this, no content
>> breakage will occur if we decide to loosen these restrictions later.
>>
>> This is the current behavior of Firefox.
>>
>> -----------------------------------------------------------
>> You are currently subscribed to public_webgl@khronos.org.
>> To unsubscribe, send an email to majordomo@khronos.org with
>> the following command in the body of your email:
>> unsubscribe public_webgl
>> -----------------------------------------------------------
>>
>