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.
All other image classes do that, and thus code generally assumes that
querying it is an immediate operation, not a monster switch over
hundreds of values. Plus this prepares for the future internal
representation that is just sizes + strides instead of the
overcomplicated PixelStorage madness.
Compared to Corrade, the improvement in compile time is about a minute
cumulative across all cores, or about 8 seconds on an 8-core system (~2
minutes before, ~1:52 after). Not bad at all. And this is with a
deprecated build, the non-deprecated build is 1:48 -> 1:41.
Neither of those are really critically important. Failure not being a
failure is fine (just don't make the shader broken in the first place),
multiple color output bindings should be in the shader source anyway.
This was quite a mess, the whole array name copypasted to each and every
access. I mean, yeah, originally I thought this would be *the* usage
pattern, but oh god this resulted in SO MANY copypaste errors.
Which ultimately means the two annoying NVidia failures in the
TextureGLTest related to pixel storage in compressed 3D images are no
longer failing as a result of cleanup.
Following the spirit of extension-based functionality, the entrypoints
are available always but do something (i.e., call the actual WebGL API)
only if the extension is advertised. Which it is only on Emscripten
3.1.66+ because older versions don't have the corresponding entrypoints,
so there it's marked as disabled.
Additionally, EXT_polygon_offset_clamp is now also working on 3.1.66+,
but there's no wrapper for it yet.
Originally, years ago, it was supplying the same input value on all
targets, and just special-cased the output on ES2 to account for the
lower bit depth. But then SwiftShader arrived, which doesn't actually
care that there's a RGBA4 framebuffer, and performs all calculations at
the full precision. Which, well, makes sense from a perf PoV, kind of, so
I adapted also the input to be quantized to 4 bits instead of 8.
BUT THEN, some Mesa update happened, or maybe it was like this always and
I just didn't realize because I was on NV cards for so long, and that
input which made SwiftShader produced the expected result, was *further
quantized*, deviating further from what was expected.
So I now ditched all that and I'm just comparing with a
sufficiently large delta. If some implementation returns exactly what was
expected for an 8-bit framebuffer even thought the framebuffer is 4-bit,
I don't care.
Some of them were marked as constexpr even though they were calling into
a deinlined function internally. Making sure the default construction is
constexpr at least, and testing that all constexpr functions actually
behave like that.
Expands the test added in 789c52fd8a, for
which a fix was done in 8f6f4053fc but
which forgot to handle the case where a buffer is unbound. The test now
fails with an invalid error.
The test added in previous commit now passes. Besides the behavior being
different for single- and multi-bind calls, an additional "interesting"
behavior is that the glBindBufferBase() / glBindBufferRange()
apparently also creates the GL object, if not already (in contrast to
the multi-bind APIs which *require* the GL objects to be created
before).
Gotta admit I wasn't aware of this side effect, at all. Once I realized
that, a fix seemed simple at first, only to make a second (re)discovery,
where glBindBuffersRange() / glBindBuffersBase() does *not* have this
side effect.
I get that it's all a shaky pile building on top of a long legacy, but
still, it never ceases to amaze.
Originally GL::hasTextureFormat() returned false on ES2 for
PixelFormat::R8Unorm, RG8Unorm, RGB8Unorm and RGBA8Unorm because
glTexStorage() didn't work with the matching Luminance, LuminanceAlpha,
RGB and RGBA formats. But since the only ES2 platform is nowadays
basically just WebGL 1, which has neither EXT_texture_rg nor
EXT_texture_storage, this implicit failure made no sense and just made
the textureFormat() (and the new genericPixelFormat() API) useless
there.
Now it maps to them, and it's up to the caller to make sure
glTexStorage() doesn't get called with those, only glTexImage does.
Furthermore, if formats from EXT_texture_rg are used, the
genericPixelFormat() now also provides inverse mapping of them back to
the generic PixelFormat. Before it was basically *no* ES2 TextureFormat
that'd work with either of these, now it's all that have a (vaguely)
corresponding PixelFormat.
An ad-hoc solution was already done in DebugTools::screenshot(), now I
need it in another place. While not as fast as the O(1) mapping from
the generic format to the API-specific ones due to the potentially
linear lookup, it definitely could be useful in general.
This one was spectacular -- ALL uses of it had also #include <tuple> in
order to std::tie() the result into separate major & minor variables. So
much compile time overhead for so little.
Not that C++ STL and exceptions would be anything to take inspiration
from, but there's std::out_of_range. Python IndexError is also specified
as "index out of range", not "bounds".
Partially needed to avoid build breakages because Corrade itself
switched as well, partially because a cleanup is always good. Done
except for (STL-heavy) code that's deprecated or SceneGraph-related APIs
that are still quite full of STL as well.
So far this was only possible by creating a temporary MeshView, while
everything else (index/vertex count, base vertex, base instance, ...)
was changeable directly on the Mesh.
Vladimír Vondruš