Ambiguity in dealing with pixel storage modes on compressed textures

[sigexp]

Currently, section 8.7 of the specs reads:

There is an ambiguity on how many rows should be skipped after obtaining the first image. "the remaining UNPACK_IMAGE_HEIGHT / b_h rows" might be read in several ways:

• remaining in regard to the point before advancing the pointer by UNPACK_SKIP_PIXELS/b_w * n + UNPACK_SKIP_ROWS/b_h * k (thus in regard to the beginning of the image). In this case, the pointer should be advanced by (UNPACK_IMAGE_HEIGHT - height - UNPACK_SKIP_ROWS) / b_h, and will end up at the beginning of the next image, hence SKIP_PIXELS and SKIP_ROWS should be applied again.
• remaining in regard to the point just before obtaining the first group. In this case, the pointer should be advanced by (IMAGE_HEIGHT - height) / b_h, and thus will end up right before the next group due to be obtained. No further applying SKIP_PIXELS and SKIP_ROWS is needed.

The second option sounds more like a correct reading. It can be illustrated with figure bellow.

This ambiguity might have caused the differences on how drivers deal with these storage modes. The test I wrote shows that Mesa handles it properly, while Nvidia proprietary driver on Linux, and Intel proprietary driver on Windows do something wrong (although, it is unclear exactly what they are doing). It's also worth mentioning that Renderdoc — which uses it's own implementation of copying pixels around for compressed textures — also handles it properly.

[pdaniell-nv]

Shouldn't the pointer (red star) on the layer 0 image be on the left side of the green rectangle rather than the right? That would obey the "without advancing the pointer" part of the quoted spec.

But to answer your question, I believe the pointer would be advanced by (IMAGE_HEIGHT - height) to get to the next layer. It's possible there are implementation bugs here, maybe due to lack of CTS coverage for this case.

[werman]

Shouldn't the pointer (red star) on the layer 0 image be on the left side of the green rectangle rather than the right? That would obey the "without advancing the pointer" part of the quoted spec.

Since it's me who made the picture in question, here is my take:

(Starting point is assumed to be on the bottom left, just to disambiguate it for anyone who is reading this.)

I think the pointer will be just over the top left of the sub-image (on the next row) since after the reading of the last sub-image's row "after which the pointer is advanced by k elements" will be applied.

But to answer your question, I believe the pointer would be advanced by (IMAGE_HEIGHT - height) to get to the next layer. It's possible there are implementation bugs here, maybe due to lack of CTS coverage for this case.

Then, is the next reading of the spec correct?

UNPACK_* parameters allow us to select a sub-cuboid in a bigger cuboid. Meaning that the sub-images in every layer are positioned all in the same way like in the image above.

If it's true then I think the wording of the spec could be improved, at least what is "remaining" in the "Then after height rows are obtained the pointer skips over the remaining UNPACK_IMAGE_HEIGHT_ / b_h rows" should be clarified. And in the best case illustration of the selection of a sub-cuboid, like above, could be added.

[pdaniell-nv]

Do you have a specific spec language change in mind to clarify this? We would be happy to review and apply to the next spec update.

Also, would you mind posting a CTS issue here describing the clearly missing CTS test coverage that needs to be added to verify this case. We'll hopefully get that fixed soon so implementations can fix their bugs.

Incidentally, do you happen to know whether IMAGE_HEIGHT is working on those broken implementations when the image is not compressed?

[sigexp]

Do you have a specific spec language change in mind to clarify this? We would be happy to review and apply to the next spec update.

Sure, we'll come up with a more explicit version and will suggest a change soon.

Also, would you mind posting a CTS issue here describing the clearly missing CTS test coverage that needs to be added to verify this case.

https://github.com/KhronosGroup/VK-GL-CTS/issues/184

Incidentally, do you happen to know whether IMAGE_HEIGHT is working on those broken implementations when the image is not compressed?

We have not tested the non-compressed cases. Moreover, the only test we've found that deals with pixel storage modes for 3D images is CTS's teximage3d_pbo.*, which doesn't even test the resulting image — just checks for errors. So, there is a high chance that those might not work as well.

[sigexp]

From section 8.5 of the spec:

Then depth two-dimensional images are processed, each having a subimage extracted as described in section 8.4.4.

I think, this implies that if a pack of consecutive layers might be seen as a cuboid, we indeed can select a subcuboid from it using pixel storage modes.

[sigexp]

@pdaniell-nv , As to the spec change, here is our suggestion:

Replace this part of section 8.7:

Then after height rows are obtained the pointer skips over the remaining UNPACK_IMAGE_HEIGHT / bh rows, if UNPACK_IMAGE_HEIGHT is positive, before starting the next two-dimensional image.

with following:

Then after height rows are obtained, if UNPACK_IMAGE_HEIGHT is positive, the pointer skips over the (UNPACK_IMAGE_HEIGHT - height) / bh rows, before obtaining the next set of blocks.

This should make it more clear, that the pack of consecutive layers is treated as a solid 3D cuboid (with layers being exactly adjacent to each other, according to the section 8.5). This way the "selecting a subcuboid" abstraction holds.

[pdaniell-nv]

Thanks. We'll review this in our next OpenGL/ES working group meeting.

[pdaniell-nv]

Discussed in the OpenGL/ES meeting and we approve the proposed clarification to the OpenGL spec. @oddhack please queue up this change for your next spec update. Thanks.

[oddhack]

This should be fixed in the spec update of May 5, 2022.