-
Notifications
You must be signed in to change notification settings - Fork 445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Trade: support mip levels in image import #369
Conversation
At first I was concerned whether Looks good to me! 👍 |
I'm interested, what exactly? Just so I can be sure that I'm doing it correctly. |
I was thinking that this might require doing per-image work per mip level. So if you'd call: for(int level = 0; level < maxLevel; ++level) {
importer->image2D(0, level);
/* this might do work maxLevel-1 times that would only need
to be done once. E.g. figure out image metadata etc. */
} But it turns out that in Basis and in DDS that part is already done in |
Yeah, then the importer would need to be more clever and cache the uncompressed data for subsequent runs (or move some work from |
f9844bb
to
0646495
Compare
Codecov Report
@@ Coverage Diff @@
## master #369 +/- ##
==========================================
- Coverage 76.46% 72.53% -3.94%
==========================================
Files 363 368 +5
Lines 17500 19330 +1830
==========================================
+ Hits 13382 14021 +639
- Misses 4118 5309 +1191
Continue to review full report at Codecov.
|
71baeb1
to
8075363
Compare
There's a new level option in each imageND() API, plus an imageNDLevelCount() query to get image level count.
Needed for caching in importers that support mip level import.
8075363
to
92a9940
Compare
For completeness, the corresponding change in plugins is mosra/magnum-plugins@15e890b...99b9e2f. It's a bit more involved in order to be efficient when loading consecutive mip levels -- scene importers have to cache an existing image importer instance to avoid decoding the image again from scratch for each level. |
There's a new level option in each
imageND()
API, plus animageNDLevelCount()
query to get image level count. The level defaults to 0, in which case it behaves exactly like before. For levels above 0, the abstract importer checks against the reported level count and asserts if it's out of bounds.Things left to do:
DdsImporter
, use the levels instead of reporting N imagesBasisImporter
(currently it opens just the base level)AssimpImporter
,OpenGexImporter
andTinyGltfImporter
pluginsimageND()
call)AnyImageImporter
movableDdsImporter
and test level propagation on those@Squareys since you're working on mosra/magnum-plugins#62, could you have a quick look at this to see if it fits your expectations or needs design changes? Thanks! 🙏