This patch is intended to fix T82349. Image that haven't been saved to disk prior to packing, don't have a filepath set, which would include the filename and file extension. If no filepath is stored for the packed image it uses only the name without file extension. This patch retrieves the extension and adds it to the name should no filepath be set.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
- Branch
- 2020-11-07-unpack-ext (branched from master)
- Build Status
Buildable 11173 Build 11173: arc lint + arc unit
Event Timeline
Comment Actions
Generally this patch seems fine.
It would be good if we could avoid loading the image just to check it's file type.
We could use ImFileType.is_a callback to detect the file-type, from the in-memory data. (see IMB_ispic_type).
Although some investigation is needed for Photoshop support, I'm not sure if there is a good reason we don't have an is_a callback for this.
Comment Actions
Update, packed PSD's don't load image data - and seeing as we can't generate PSD from within Blender, I think it's reasonable to accept this limitation.
Comment Actions
The IMB_ispic_type_from_memory appears to return an incorrect imtype under certain circumstances or I'm using it wrong. When an texture is created within Blender using the 32-bit float option and this file is packed into the *.blend file, then IMB_ispic_type_from_memory identifies this as .tga when it should be .exr. I'll have to investigate this first, before updating the patch.
Comment Actions
I've found the issues. They were both in my code and in IMB_ispic_type_from_memory. Will update the patch shortly.
Comment Actions
Previously IMB_ispic_type_from_memory didn't set the buffer when mem_size >= HEADER_SIZE, thus attempting to identify the file type on the default values of the array. Since the buf_static should be filled with the header bytes in both cases, we can just limit the memcpy to the smaller one of header size and input memory size. My patch doesn't use the separate pointer, because the array should decay to a pointer. However, if this is against code style and a const pointer is preferred I can update the patch.
Additionally, I made a mistake myself. The returned type by IMB_ispic_type_from_memory is the ftype not the imtype, thus the call to BKE_image_ftype_to_imtype is still necessary.
| source/blender/blenkernel/intern/packedFile.c | ||
|---|---|---|
| 633–642 | This could be moved into unpack_generate_paths at the end of the block containing the comment: /* Note: we do not have any real way to re-create extension out of data... */ Otherwise anyone someone reading this function may assume extensions aren't being handled when in fact they are handled outside of the function. | |
| source/blender/imbuf/intern/util.c | ||
| 156 ↗ | (On Diff #30986) | Changes here are no longer needed. |
| source/blender/blenkernel/intern/packedFile.c | ||
|---|---|---|
| 633–642 | unpack_generate_paths is used for more than just images, hence I didn't put the implementation there. It's also used in BKE_packedfile_unpack_vfont, BKE_packedfile_unpack_sound, BKE_packedfile_unpack_volume. I don't think this would be a good idea, since we would have to give image input a special treatment in this function. The note is still correct, the function cannot generally retrieve the extension from the data, which is why we supply it as part of the name argument to this function. This is already the case in the current implementation in master, when imapf->filepath is set. | |
Comment Actions
unpack_generate_paths can check for image data, eg: GS(id->name) == ID_IM), it's already checking ID type to detect the sub-directory to save files to.
Comment Actions
Thanks, committed rB9a73417337f5dffa08f893382acc29191d8405f6 with minor edits (don't add extension if the file magic isn't known).