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.
With ARB_DSA disabled, the test added in previous commit would assert
already during bind(), and not later in label(). This was due to the
state tracker being updated too early here.
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.
I'm giving up there. Both Clang-cl and MinGW GCC report a far bigger
set of lines than their Linux / Mac counterparts (block ends, lines
containing just `else`, etc etc.), which wouldn't be a problem on its
own, if those lines would be covered. But on Windows there isn't a
single CI job that runs through GL/Vk codepaths -- as Mesa isn't easy to
get there and SwiftShader is utter crap -- which means there are extra
lines in GL/Vk codepaths that are reported as uncovered from Windows CI
jobs, while the same lines on Linux and Mac arere excluded and thus
there isn't anything that could make them green as well.
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?
Vladimír Vondruš