Page MenuHome

Fix Wrong image_user frame offset when opening image with Detect Sequences and filename ends with a digit
AbandonedPublic

Authored by Vincent Blankfield (vvv) on Jul 6 2020, 11:18 PM.

Details

Summary

This is a follow up on T78537, similar issue, but with less severe consequences, as T78646 (D8213). Both these issues were causing the problems the original bug reporter had in their test file, which was renderer allocating enormous amount of image slots during the animation render with persistent images. Both of them were caused by incorrect image_user settings, that for different reasons got into the file.

Currently, when opening an image with Detect Sequences enabled (which is on by default and hidden in the N-panel), the image_user.frame_offset will be set to a trailing number in the image filename (if it ends with a number). This seems pretty harmless, except in some obscure cases involving mixing up manually added images with the same images imported by a script. The script imported images will have the correct frame_offset, and the manually added ones may have it incorrect. Consequently, current_frame will differ. In turn, this leads to allocating different image slots for rendering in each case of the same image with different frame_current. Not that critical, and happens in really obscure cases, but could be avoided by not having wrong frame_offet values.

This issue can be simulated by running this script:

import bpy
import os

simulateTheIssue = True
# Path should exist, filename should end in a number
path = os.path.expanduser("~/Desktop/test.123.png")

# Cleanup
for mat in bpy.data.materials:
    bpy.data.materials.remove(mat)
for ob in bpy.data.objects:
    if ob.type != 'CAMERA' and ob.type != 'LIGHT':
        bpy.data.objects.remove(ob)
for im in bpy.data.images:
    bpy.data.images.remove(im)

# Same image for both materials
img = bpy.data.images.load(path)

# Simulate imported material (correct frame_offset)
mat1 = bpy.data.materials.new("Imported")
mat1.use_nodes = True
tex_node1 = mat1.node_tree.nodes.new("ShaderNodeTexImage")
tex_node1.image = img
mat1.node_tree.links.new(tex_node1.outputs[0], mat1.node_tree.nodes[1].inputs[0])
bpy.ops.mesh.primitive_cube_add(location=(-1, 0, 0))
bpy.context.active_object.data.materials.append(mat1)
# T78646 workaround (that's another story)
tex_node1.image_user.frame_duration = 1

# Simulate a material with manually added texture (incorrect frame_offset)
mat2 = bpy.data.materials.new("Manual")
mat2.use_nodes = True
tex_node2 = mat2.node_tree.nodes.new("ShaderNodeTexImage")
#tex_node2.image = bpy.data.images.load(path)
tex_node2.image = img
mat2.node_tree.links.new(tex_node2.outputs[0], mat2.node_tree.nodes[1].inputs[0])
bpy.ops.mesh.primitive_cube_add(location=(1, 0, 0))
bpy.context.active_object.data.materials.append(mat2)
# T78646 workaround (that's another story)
tex_node2.image_user.frame_duration = 1
if simulateTheIssue:
    # Simulate manually added image with sequence detection
    tex_node2.image_user.frame_offset = 123

# Need to be packed for the excessive render slot issue to happen
bpy.ops.file.pack_all()

# Print image_user parameters
for mat in bpy.data.materials:
    if mat.use_nodes:
        for nd in mat.node_tree.nodes:
            if nd.type == "TEX_IMAGE":
                print(f"Image: {nd.image.name}")
                print(f"  frame_offset: {nd.image_user.frame_offset}")
                print(f"  frame_start: {nd.image_user.frame_start}")
                print(f"  frame_duration: {nd.image_user.frame_duration}")
                print(f"  frame_current: {nd.image_user.frame_current}")

In case of simulateTheIssue = True the scene will be rendered using 2 image slots. In case of simulateTheIssue = False - only 1. The images need to be packed, because in case of not packed ccl::ImageManager::add_image(filename, params, tiles) will use ccl::OIIOImageLoader which in equals() compares only file paths, and in case of packed ccl::ImageManager::add_image(loader, params) will use ccl::BlenderImageLoader which compares image and current frame. And in ccl::ImageManager::add_image_slot a false comparison will lead to allocating of a new slot:

if (img && ImageLoader::equals(img->loader, loader) && img->params == params) {
  img->users++;
  delete loader;
  return slot;
}
// otherwise allocate a new slot

The loader frame comparison seems to be from rB6cf4861c3ac0.

Diff Detail

Repository
rB Blender

Event Timeline

Vincent Blankfield (vvv) requested review of this revision.Jul 6 2020, 11:18 PM
Vincent Blankfield (vvv) created this revision.

Offset is allowed to be larger than length. For example if you want to play frame 100 to 110, you would have offset 100 and length 10.

I think the fix I'm proposing in D8242: Do not advance the image_user current frame for single image files will solve this issue.