So line code coverage reports which branches weren't taken by tests.
Other libraries need similar treatment but there's too many cases to fix
so I'm doing it just here for now.
This happened to me from time to time and I always treated it as user
error. But when thinking more about it, calling wrap() without actually
knowing if the object was already created or not, is a completely valid
use case, especially in cases of making non-owning references to
existing GL objects that might only get created at some point after.
These tests now cause assertions in all cases on GLES, on desktop
some only if ARB_direct_state_access is disabled. In all cases the
assertion is triggered by querying object label (which is a no-op if no
debug extension is present, but it calls createIfNotAlready() always),
because that's the simplest, the real-world cases are usually some
texture bind() calls.
Fix in the next commits.
Yet another broken behavior with compressed textures on NVidia unearthed
by the recent changes with compressed block properties being set almost
always.
They are all using desktop-only APIs and most of the compressed variants
likely suffer from the same NVidia bug, so it's useful to have them
next to each other.
It's amazing how much functionality is there that isn't really tested.
These new tests will likely fail on macOS which doesn't support
ARB_compressed_texture_pixel_storage, and, in particular, they likely
also failed before, just in rarer cases, in particular only when
- either a compressed image up/download was made with no storage
parameters but block size set,
- or compressed up/download after a Context::resetState() call,
regardless of whether any storage parameters or block size being used
While the ES spec seems to say that these are all ignored when uploading
a compressed image (and so resetting them shouldn't be needed), with a
WebGL 2 build Chrome is complaining that the pixel unpack parameters are
invalid if they're not explicitly reset to zero before the compressed
upload.
The properties are now always taken from the format itself. This
also reverts commit a96fc85736, as it's
now again possible to pass an empty image with non-default storage
properties to image query APIs.
With the recent changes where compressed image block properties no
longer need to be queries from GL, we also no longer need to query
GL_TEXTURE_COMPRESSED_IMAGE_SIZE, and thus we don't need the NV-specific
workaround where the returned value was sometimes broken for cubemaps.
With this change, neither GL is queried for compressed block size
properties nor they're taken from CompressedPixelStorage anymore. For
image upload they're taken directly from the passed image and set to
GL's pixel pack state, for image download they're taken from known
GL::CompressedPixelFormat properties set to GL's pixel unpack
state and saved to the image.
Besides removing a bunch of checks from tests, there isn't anything new
to *add* there -- everything should work just as before, or
(assuming shitty drivers with broken format queries) better.
Also, in some cases the internal format queries were made into a
zero-initialized output variable to ensure consistent behavior with
broken drivers. That's now done in all cases, with a lengthy comment to
ensure this doesn't get "cleaned up" by accident.
Once the block sizes are implicitly taken from known format properties
and supplied to GL instead of being queried from GL or required to be
set by the user in CompressedPixelStorage, this commit will be reverted.
Similarly like pixel size is stored in uncompressed images, block size
makes it possible to perform size checks on passed data, slice the images
and so on. It only took over a decade to get that done.
The block properties coming from CompressedPixelStorage are currently
expected to be either not set at all or exactly match what's stored in
the image for given format. The PixelStorage will get eventually
deprecated in favor of a simpler and more flexible representation, but
that's another big chunk of work so it's first done like this.
The GL library tests currently blow up on various assertions and it
isn't yet updated to make use of the known format properties instead of
querying them from GL. That'll be done in the following commits.
In case of compressed images the tests were absolutely insufficient, as
they were always verifying just a single block, and thus they passed
even though the actual calculation was wrong in several ways.
And then the occupiedCompressedImageDataSize() wasn't tested at all,
even though it should return something completely different from the
function it delegates to.
Fixes to both in the next commit.
Unpack skip / row height parameters aren't supported there but alignment
is, so be sure to test that. Together with discovering some extremely
weird behavior, likely some shitty ANGLE bug.
This likely originates in pre-2018 state of the repo where the GL
wrapper was part of the main library and CompressedPixelStorage wasn't a
thing on ES simply because there was no such extension.
This was quite nasty, a multi-day effort to trim this down and then
increasingly growing disappointment as I discovered it was affecting
basically any use of the API.
Less error-prone because of full type safety and much nicer overall. The
test code originates from a time before slice-to-a-member-function was a
thing and then it got just copied everywhere without much thought.
For integer attributes, that is. This never worked and the MeshGLTest
even had an explicit test case for this, but it took me until now to
realize that this would be best caught at compile time. So now it is.
But since this functionality dates back to 2010, just removing the
values would likely break a lot of code (well, broken code, but still),
so the disallowed values are now deprecated, giving a compile time
warning when used, and are removed only on non-deprecated builds.
And of course, besides the already-existing workaround for NV where it
reports size in bits instead of bytes, Mesa reports completely broken
pixel sizes in some cases. For DX1 I get 2x2 (?!), for ASTC 10x10 I get
1x1, which is basically unfixable.
I wonder how at all these utilities were used internally with success for
so long. Or were they never used because in every case I ended up
supplying the properties manually?
IIRC this still failed when I did the last changes to these tests in
October 2024 in e9b07ef4ce, so it seems
that the driver got fixed since. There are still the broken cases in
Texture3D tho, updated in the previous commit.
Not sure what was happening here, but my suspicion is that I took what
NVidia did with BPTC when used in 3D textures as the correct thing, and
then tried to shape the test around that. Or something.
Basically, instead of the blocks being 4x4x1 as the format says, the test
overwrote the pixel storage parameters to be four times larger 4x4x4
blocks, and then went with that, trying to shape the other test variants
to match that as well. And failing really bad, because it failed on Mesa,
and the only case where where it matched the (wrong) expectation was on
NVidia and only when the custom (wrong) pixel storage was used.
The uncompressed variant preserves the buffer contents if passed an empty
view, which was done in 5ee461bdbe (2015!)
but no corresponding change was ever done for the compressed variant. So
let's make both do the same, and actually test the code path, which
wasn't done until now either.
For compressed images the expected message was completely wrong, meaning
it'd fail in any case, and they were missing a SKIP on no-assert builds.
Then, for GL::[Compressed]BufferImage the assertions in setData() were
never tested at all.
They're not parsed since 6b22a11170
(2020), so there's no point in keeping those workarounds. They're only
kept in utility application sources as they're parsed for pages, and in
tweakable implementations where it's easier to just copypaste the whole
ifdef expression from the header every time instead of modifying it to
not include DOXYGEN_GENERATING_OUTPUT.
Vladimír Vondruš