Page MenuHome

Do not advance the image_user current frame for single image files

Authored by Vincent Blankfield (vvv) on Jul 7 2020, 10:46 PM.



This is a continuation (hopefully the last one) on D8213 and D8228. In short, an image_user may be created with wrong settings: frame_duration = 100 by default from Python API (D8213); frame_offset equal to trailing number in a filename of an image, that is opened from UI with Detect Sequences enabled (D8228). These incorrect settings may lead to excessive memory usage when rendering with cycles (in specific conditions). The "excessive" may vary from allocating one more image, to tens of gigabytes and inability to render simple animation.

While D8213 and D8228 attempt to fix the issues of setting the incorrect values for image_user, those fixes will not fix the situation for the users that already have files with the messed up settings (such files may have been created by using popular import scripts). The users may not even be aware of this, when their animations will render using tens of gigabytes of memory, while really needing an order of magnitude less.

The root of the issue lies in ccl::ImageManager::add_image_slot that will allocate a new image if ImageLoader::equals(img->loader, loader) && img->params == params evaluates to false. And for the packed images ccl::BlenderImageLoader is used, that defines equals() as b_image == other_loader.b_image && frame == other_loader.frame. So a new image will be allocated when the image_user frame is different.

This patch proposes not to advance the image_user frame at all if the image isn't animated (aka source isn't a movie or a sequence). This will retrofix the rendering problem for the existing files and make rendering not vulnerable to the wrong image_user settings. The frame settings will be ignored for single image files.

I've tested this with some combinations of packed, not packed, animated, static textures, but still not completely sure I've covered everything. Also I'm not sure about the reason why the frame is currently calculated and compared for all types of images. So this is intended not really as a solid fix, but at least to bring some attention to the problem in case this fix can't be applied. I would've filed a bug report, but this can't really be reproduced without a "corrupted" file, and makes no sense out of context. Also sorry for spamming 3 diffs for one problem. I think the issues are completely different, the only commonality between them is one particular problem, that may arise while rendering with specific conditions. And as they are different, some of the fixes may be acceptable, while others not. If this should be one diff and each of the three is acceptable, I can join them if needed. Or feel free to abandon them and do whatever is needed I can't commit anyway.

Diff Detail

rB Blender
Build Status
Buildable 9039
Build 9039: arc lint + arc unit

Event Timeline

Vincent Blankfield (vvv) requested review of this revision.Jul 7 2020, 10:46 PM
Vincent Blankfield (vvv) created this revision.

I think this is close to the right fix. But when the image is not animated, it should set the current frame number to something fixed. Otherwise when changing the image type from sequence to still it will have a nonsensical value.

Set current frame to fixed value for still images.

This revision is now accepted and ready to land.Jul 16 2020, 3:50 PM