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.
Not sure why this wasn't a problem in Corrade, maybe because here the
clang-cl build doesn't run GL tests and so while there the } lines were
marked by it as covered (instead of being ignored), here they're marked
as uncovered and all other jobs ignore them?
That just makes no sense, as one then has to explicitly call disable()
to not have printStatistics() produce a useless line in the output.
Also have the same behavior also if it's constructed or set up from an
empty list of measurements, since that should be equivalent.
This makes the test added in previous commit pass on Chrome. This also
fixes the UI library there now, before it didn't show icons exactly due
to this, because they're uploaded with a non-zero offset (after text
glyphs) and Chrome requires the image height to be set in that case even
if uploading to the very first slice.
It works everywhere except on Chrome WebGL 2, which complains that pixel
unpack parameters are incorrect. Which is a very helpful error on its
own, fortunately I know already where the problem is.
Like, no wonder C++ codebases are so bug-ridden, given all APIs in the
standard library are weird like this, requiring the user to completely
understand their inner workings in order to be able to use them safely.
A fix for this got made in 390d3fc06f
(2022) already, but somehow the corresponding tests were living in a
half-done stash ever since. I should probably also just do my own binary
search thing so I don't need to test everything so extensively.
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 means SwiftShader is only used for Mac ES3 build and Android.
Also remove the parallelism and --output-on-failure, it's unhelpful as
it only leads to less information being provided if the CI fails.
I thought this would fix a leak reported by ASan when running on Mesa 25
llvmpipe, but it didn't. Given that no leak is reported when *not*
running on llvmpipe, I assume this is llvmpipe's fault.
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.
Vladimír Vondruš