Not only the implementation is a template hell full of its own problems,
the internals are also just wrapping trivial bits of STL without adding
anything of value on top, so ultimately any code that actually used this
thing ended up being slower to build, more verbose, harder to reason
about and with thousands of tiny little inefficiencies all over the
place. So let's just finally say no to this idea from 2012.
"These seemed like a good idea at the time." But that was in 2013 and
while that might have been acceptable back then, it's not anymore, it's
all just a complex abstraction that prevents any reasonable resource
reuse and forces a dedicated draw call for each use.
Just do this directly from the corresponding Primitives instead.
Deprecating those allows me to eventually deprecate the (also
overengineered) ResourceManager class, at which point Magnum should be
finally free from the worst design decisions from 15 years ago.
Unfortunately several examples still rely on it (while it only makes
them more complex, not better), have to wait until those are cleaned up
first.
I'm going to deprecate the (awfully inefficient)
DebugTools::ForceRenderer and ObjectRenderer, and need at least
something else to direct the users to. This should suffice.
This was done back in October 2024 for all plugins in the magnum-plugins
repository, here it was blocked by usage in (now deprecated) MagnumFont.
In comparison the AnyImageImporter etc. plugins *are* meant to be
constructed directly by plugins that depend on them, so for those
nothing changes.
With StbTrueTypeFont no longer being embarrassingly slow due to a stupid
error, the MagnumFont plugin doesn't really provide any advantage
anymore. Originally I thought I'd update it to not be forcibly relying
on a TGA format and maybe add kerning support, but the image + text
format is extremely inefficient compared to basically anything else and
so I don't see a point in supporting it further. Marking it as
deprecated should also hint to people that using this thing is not a
good idea.
The MagnumFontConverter plugin was also the only user of the
AbstractFontConverter APIs, which right now are the last that didn't go
through a STL cleanup process. I plan to make a converter plugin for
font "minification" using HarfBuzz subsetting functionality, and not
having to support anything else that relies on the outdated APIs will
make the font converter API updates a bit easier.
Finally, the MagnumFont plugin was also the last one that directly
instantiated the TgaImporter plugin, instead of going through
AnyImageConverter. Marking it deprecated allows me to deprecate direct
instatiation in the remaining plugins as well.
Because MeshTools::removeDuplicates() uses a STL hashmap, it's quite
bad. So bad that on the Rungholt scene it takes 4 seconds out of 7.3 for
the whole import process.
Now I'm able to measure -- with the Rungholt scene from McGuire
archives, it takes it a whooping 7.3 seconds, measured with
magnum-sceneconverter --info-meshes --profile rungholt.obj
Compared to that, AssimpImporter in the default configuration takes 10.3
seconds, but with
magnum-sceneconverter --info-meshes --profile -I AssimpImporter \
-i postprocess/JoinIdenticalVertices=false,postprocess/SortByPType=false \
rungholt.obj
it takes just 4.3 seconds. And has the same vertex data size in total,
so that should probably be the default. I got some work to do, then.
Basically the same logic as with Trade::AbstractImporter::openMemory(),
the doOpenData() signature is (yet again) changed to accept a pair of a
r-value Array reference and a DataFlags describing whether the memory is
owned or not. Compared to Trade, I don't anticipate any in-place
mutation of open fonts, so there's no DataFlags::Mutable.
There's also no DataFlags::Global because data of the opened font aren't
exposed further anywhere (so far at least, but I can imagine SVG data
eventually possibly passed through to importers, which then could be a
use case for this flag).
This is the final breaking change to the AbstractFont doOpenData() API.
Adapting of all other code to these changes will be done in a subsequent
commit, so right now a non-deprecated build still fails.
The AbstractFont::openFile() and openData() now get an additional
argument specifying a font index to allow picking a concrete font index
for example in a TTC collection.
This index has to be specified upfront with no possibility to change it
afterwards, because that's how both FreeType and stb_truetype work. Thus
there are also new fileFontCount() and dataFontCount() APIs taking a
filename / data argument, instead of this being a fontCount() query on
an opened file. The implication from this is that -- unlike basically
all other APIs in Trade and elsewhere -- specifying an out-of-bounds
font index isn't an assertion (i.e., a programmer error) but rather a
graceful failure, because requireing the user to first call
fileFontCount() and then openFile() would mean having to open and
parse the same file twice, which is undesirable overhead.
While this isn't breaking for end users, as it's just a new optional
argument to openFile() and openData(), it's a breaking change for plugin
interfaces. The old doOpenFile() and doOpenData() interfaces are still
there to help transitioning existing plugins, but are marked as
deprecated. Delegating to those turned out to be quite involved, so
there are many new tests verifying this compatibility code path.
The plugin interface string version is bumped but in the next commits
I'm making use of the doOpenData() / doOpenFile() API breakage to do
more changes. Those won't result in further plugin interface changes, so
this set of commits should be treated as a single indivisible change to
the plugin interface. For this reason, uses of AbstractFont outside of
AbstractFontTest (including the MagnumFont plugin) aren't adapted yet
-- they pass tests as before, but compile only with deprecated APIs
enabled.
Co-authored-by: Andrew Snyder <asnyder@minitab.com>
Somehow I overlooked this extension back when implementing multidraw, or
maybe it was intentional because ANGLE supported multidraw but not this
extension.
Causes the following warning on GCC on a static build if user code isn't
compiled with -fvisibility=hidden:
warning: ‘Magnum::GL::Mesh’ declared with greater visibility than
the type of its field ‘Magnum::GL::Mesh::_attributes’ [-Wattributes]
According to quick testing, given that the struct definition isn't
public, presence of the MAGNUM_GL_LOCAL has no effect on the symbol
being exported. On the contrary, inner classes that are meant to be
exported (such as the ones in GL::Framebuffer) have to contain
MAGNUM_GL_EXPORT, otherwise it leads to linker errors. Furthermore, all
other inner structs with local definitions holding PIMPL state and such
don't have a MAGNUM_*_LOCAL macro applied anywhere
Thus the macro was likely redundant, and is removed.
I don't think my ancient Doxygen fork is aware of [[nodiscard]], and
it's quite clear that the return value is important when looking at the
docs, so just remove it from processing.
I thought I went over all these several years ago already, but
apparently not or maybe back then not all websites were HTTPS-ready. Now
they mostly are, except for maybe one or two.
I need this to sample a color map for debug visualization in the UI
library. Thus so far it's just 1D, and with 8-bit input. Other variants
might get added in the future if needed.
Strangely enough, that C4312 warning suppression now needs to be C4834
instead while I'm pretty sure it was correct back in March 2024 when I
added this in 4f7b57ffd6. C4312 is however
for "conversion of A to B of greater size", and in
4435877cf1 I was fixing a lot of those, so
maybe I just used the same number by accident and because this
particular warning is extremely easy to miss, it was never correct in
the first place? Who knows.
Yet another broken behavior with compressed textures on NVidia unearthed
by the recent changes with compressed block properties being set almost
always.
Text only, SDL3 has support for arbitrary mime types so the API is named
clipboardText() and not just clipboar(). Wanted to add this for
Emscripten as well, but after looking around a bit, I don't think I want
to deal with that until someone actually wants clipboard support
somewhere.
Is critically needed for the GltfImporter now, as the per-data importer
state is not useful on its own anymore and needs the Utility::Json
instance as well. And doing the same for AnyImageImporter as well for
feature parity.
Vladimír Vondruš