Details
Diff Detail
- Repository
- rB Blender
Event Timeline
The changes look good to me, and I confirmed that this fixes the issue loading UDIM textures that have only one tile (as described in D13297). Thank you for the quick fix!
As I noted in D13297#399515, I don't agree with the "two consecutive frames means UDIM" heuristic.
I didn't mean to disregard your previous comment about this, Sybren. I verified that this patch fixes the issue in the USD import code, but I should have clarified that I don't have enough context to evaluate full implications of this change. @Jesse Yurkovich (deadpin) can you clarify how this heuristic is applied? Is this behavior that users opt into?
@Sybren A. Stüvel (sybren) I don't much like it either but it's providing a barrier against the current defaults. Let me try to explain the current system.
UDIM detection currently goes as follows:
- If the "UDIM detection" property is True for the image.open operator (by default it is True)
- AND the files contain a number in its name between 1001 and 2000 (inclusive)
- AND there's no file detected outside the range
- AND (there's at least 2 such files that share that filename pattern OR a tokenized filename was provided after this patch)
Then it will load as a UDIM tile set.
The "2 files" heuristic is primarily guarding against the fact that "UDIM detection" is True by default and users can run into something unexpected.
The downside of a "real" UDIM file "failing" to be marked as a UDIM if only 1 file was found is the following:
- The image will still be loaded but as IMA_SRC_FILE (still usable)
- The user can change the source to IMA_SRC_TILED in the Image Editor and type the tokenized filename directly to "fix"
If we chose to default the "UDIM detection" property to False this problem would basically disappear. It would then be a conscious effort by users to switch to UDIM loading.
This carries some downsides though:
- The image open properties panel is not opened by default (bad for discoverability and re-learning from what's been the default from 2.82) and when it is open it takes up space
- For drag n'drop we cannot auto-open the Redo panel so folks will have to become accustomed to opening that panel to set the UDIM detection option
Conversely, if "regular" users learn to turn that option off before loading their files, then we wouldn't have an issue either.
I still feel that Blender shouldn't guess what users mean.
If we chose to default the "UDIM detection" property to False this problem would basically disappear. It would then be a conscious effort by users to switch to UDIM loading.
That's a lot better IMO. Artists know best when they want to load UDIM and when they just want to load a single image.
- The image open properties panel is not opened by default (bad for discoverability and re-learning from what's been the default from 2.82) and when it is open it takes up space
- For drag n'drop we cannot auto-open the Redo panel so folks will have to become accustomed to opening that panel to set the UDIM detection option
This can be addressed. Having a new option with a new feature doesn't look like a huge thing to me. Also, when Blender incorrectly decides that a file is a UDIM tile, people will also have to re-learn what's going on, but then there is no clear checkbox they can ask for help about. It's just Blender doing something weird without explanation.
I think Blender guessing what users mean is fine in the case of opening a single image. These kinds of heuristics can be nice quality of life improvements, and we have already done this for image sequences for a long time.
In the context of loading USD files relying on such heuristics is not going to work reliably though. Don't USD files already have file paths with <UDIM> tokens? I think that token is all the USD importer should be looking at, none of the other heuristics should be used for USD, probably not even as an option.
Indeed, in D13297 the USD importer is checking for the <UDIM> token and calls BKE_image_get_tile_info() for files that contain the token. BKE_image_get_tile_info() also checks for UDIM tokens internally to determine if single UDIM files are allowed.
Even if zero UDIM tiles exist on disk, I think the image could still be set to IMA_SRC_TILED when <UDIM> is in the filepath. It seems best to preserve that info even if image files are missing. Because that might only be temporary or manually fixed by the user.
I'm a bit unclear on ultimate path forward for some points above:
- [Yes or No] Change the existing "UDIM detection" boolean from True to False (note: This boolean is remembered for the duration of the current blender session; it's reset back to default on loading another file though)
- [Yes or No] Change the image.open dialog to open its Properties panel by default to make users more aware of the loading options
Regardless of the above, I will remove the restriction to have 2 or more files as part of the loading process.
I think this patch is fine by itself, just should not be considered a complete fix for the USD case.