Details

Reviewers

Commits

rB0c3b215e7d54: Images: refactor how failed image load attempts are remembered

Summary

This removes ImageTile->ok and ImageUser->ok.
They had three possible values which (probably) had the following meanings:

0: There was an error while loading the image. Don't try to load again.
1: Default value. It will try to load the image.
2: The image was loaded successfully.

This image-wide flag did not make sense unfortunately, because loading may work for some frames of an image sequence but not for others.

My current understand is that the ->ok flags only existed as an optimization to avoid checking the file system over and over again when a file is missing.
To get this optimization back, the patch is changing MovieCache so that it can also cache failed load attempts. As a consequence, ibuf is allowed to be NULL in a few more places. I added the appropriate null checks.

This would be useful to avoid a hacky solution in D12827.

Diff Detail

Repository

rB Blender

Branch

remove-image-ok (branched from master)

Build Status

Buildable 18088
Build 18088: arc lint + arc unit

Event Timeline

Jacques Lucke (JacquesLucke) requested review of this revision.Oct 21 2021, 7:33 PM

Jacques Lucke (JacquesLucke) created this revision.

Harbormaster completed remote builds in B18088: Diff 43722.Oct 21 2021, 7:33 PM

Jacques Lucke (JacquesLucke) retitled this revision from Images: Remove image ok flag (WIP). to Images: Remove image ok flag..Oct 21 2021, 7:46 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)

Jacques Lucke (JacquesLucke) added a reviewer: Brecht Van Lommel (brecht).

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Oct 21 2021, 7:49 PM

remove dead code

Harbormaster completed remote builds in B18089: Diff 43723.Oct 21 2021, 7:57 PM

This is an important optimization, that if missing can cause Blender to hang repeatedly loading corrupt files. We should not remove the flag without an alternative solution.

Alright, will try to find the places where loading can fail and add back the optimization.

I think the better solution would be to add NULL entries to the image cache when the file failed to load, but I have not looked into all the implications of the changes in this patch.

Merge branch 'master' into remove-image-ok
quiet warning

Harbormaster completed remote builds in B18120: Diff 43767.Oct 22 2021, 1:13 PM

bring back optimization by also caching failed load attempts

Harbormaster completed remote builds in B18122: Diff 43769.Oct 22 2021, 1:21 PM

Jacques Lucke (JacquesLucke) edited the summary of this revision. (Show Details)Oct 22 2021, 1:24 PM

Jacques Lucke (JacquesLucke) retitled this revision from Images: Remove image ok flag. to Images: Cache failed load attempts in MovieCache instead of in `ok` flag..Oct 22 2021, 1:31 PM

Jacques Lucke (JacquesLucke) mentioned this in D12827: Geometry Nodes: Add Image Texture node..Oct 24 2021, 10:34 PM

Jacques Lucke (JacquesLucke) mentioned this in rB039094c1ec50: Geometry Nodes: new Image Texture node.Oct 25 2021, 1:04 PM

I tried to carefully review the code and tried to break it with invalid image files etc, seems to be working fine.

Nice to get rid of this finally.

This revision is now accepted and ready to land.Nov 1 2021, 8:01 PM

Closed by commit rB0c3b215e7d54: Images: refactor how failed image load attempts are remembered (authored by Jacques Lucke (JacquesLucke)). · Explain WhyNov 2 2021, 11:17 AM

This revision was automatically updated to reflect the committed changes.

Jacques Lucke (JacquesLucke) added a commit: rB0c3b215e7d54: Images: refactor how failed image load attempts are remembered.

Images: Cache failed load attempts in MovieCache instead of in `ok` flag.
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 43722

source/blender/blenkernel/BKE_image.h

source/blender/blenkernel/intern/camera.c

source/blender/blenkernel/intern/image.c

source/blender/blenkernel/intern/image_gpu.c

source/blender/blenkernel/intern/node.cc

source/blender/blenkernel/intern/object.c

source/blender/blenkernel/intern/screen.c

source/blender/blenkernel/intern/texture.c

source/blender/blenloader/intern/versioning_260.c

source/blender/blenloader/intern/versioning_280.c

source/blender/blenloader/intern/versioning_legacy.c

source/blender/compositor/operations/COM_ViewerOperation.cc

source/blender/draw/engines/gpencil/gpencil_draw_data.c

source/blender/editors/object/object_bake_api.c

source/blender/editors/render/render_internal.c

source/blender/editors/render/render_opengl.c

source/blender/editors/render/render_preview.c

source/blender/editors/sculpt_paint/paint_image_2d.c

source/blender/editors/space_image/image_buttons.c

source/blender/editors/space_image/image_undo.c

source/blender/makesdna/DNA_image_types.h

source/blender/makesrna/intern/rna_image_api.c

source/blender/nodes/composite/nodes/node_composite_image.cc

source/blender/nodes/composite/nodes/node_composite_splitViewer.cc

source/blender/nodes/composite/nodes/node_composite_viewer.cc

source/blender/nodes/texture/nodes/node_texture_image.c

Images: Cache failed load attempts in MovieCache instead of in `ok` flag.ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 43722

source/blender/blenkernel/BKE_image.h

source/blender/blenkernel/intern/camera.c

source/blender/blenkernel/intern/image.c

source/blender/blenkernel/intern/image_gpu.c

source/blender/blenkernel/intern/node.cc

source/blender/blenkernel/intern/object.c

source/blender/blenkernel/intern/screen.c

source/blender/blenkernel/intern/texture.c

source/blender/blenloader/intern/versioning_260.c

source/blender/blenloader/intern/versioning_280.c

source/blender/blenloader/intern/versioning_legacy.c

source/blender/compositor/operations/COM_ViewerOperation.cc

source/blender/draw/engines/gpencil/gpencil_draw_data.c

source/blender/editors/object/object_bake_api.c

source/blender/editors/render/render_internal.c

source/blender/editors/render/render_opengl.c

source/blender/editors/render/render_preview.c

source/blender/editors/sculpt_paint/paint_image_2d.c

source/blender/editors/space_image/image_buttons.c

source/blender/editors/space_image/image_undo.c

source/blender/makesdna/DNA_image_types.h

source/blender/makesrna/intern/rna_image_api.c

source/blender/nodes/composite/nodes/node_composite_image.cc

source/blender/nodes/composite/nodes/node_composite_splitViewer.cc

source/blender/nodes/composite/nodes/node_composite_viewer.cc

source/blender/nodes/texture/nodes/node_texture_image.c

Images: Cache failed load attempts in MovieCache instead of in `ok` flag.
ClosedPublic
Actions

Revision Contents
Changeset List