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.
And make resetState() aware of ARB_compressed_texture_pixel_storage so
when it's not supported, it doesn't cause the very next compressed image
upload or download to fire a GL error due to an unknown state being set.
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.
The tests now pass, but this damn GL pixel storage API is so convoluted
with so many redundant and mutually conflicting degrees of freedom that
it's impossible to be sure. Looking forward to when I can finally drop
that thing and calculate it on the fly from just size + stride, like it
should have been from the start.
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.
It's used only there and only to supply a silly argument to a broken API.
Unfortunately back when adding this utility in 2015 I didn't document
what was it for, which initially made me think it's there for some
suspicious reason. Well, the reason is suspicious, but for an entirely
different reason.
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.
All places were already consistently using "expected to <stuff>"
describing expectations backed by assertions, except for the Primitives
library. Fix that, plus random other doc fixes.
The MAGNUM_GL_ABSTRACTSHADERPROGRAM_SUBCLASS_DRAW_IMPLEMENTATION() and
MAGNUM_GL_ABSTRACTSHADERPROGRAM_SUBCLASS_DISPATCH_IMPLEMENTATION() *is*
documented so that one stays not underscored.
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.
Vladimír Vondruš