Image: Don't flush GPU textures during undo.
AbandonedPublic
Actions

Authored by Jeroen Bakker (jbakker) on Aug 17 2021, 2:11 PM.

Details

Reviewers

Maniphest Tasks

T90593: Undo takes several seconds just to move default cube within scene containing panorama if viewport shows materials

Summary

Currently GPU textures are flushed as the image data aren't identical
compared to the previous version in the undo stack.

This patch will clear the attributes that are cleared during loading.
What would make the structures identical. It still has some issues as
the data are still being considered not identical.

Note: contains some debug utilities to view the exact data that is saved
and read.
When enabling IMAGE_UNDO_HEX_DUMP the consoles shows the data in (at
least for me) readable way.

When writing the ID of the image for the undo-stack.

00000000: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00000020: 00 00 00 00 00 00 00 00  49 4d 62 61 63 6b 67 72  6f 75 6e 64 2d 63 6f 6d  70 72 65 73 73 65 64 2e          IMbackground-compressed.
00000040: 6a 70 67 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  jpg
00000060: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  01 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00                  .
00000080: 00 00 00 00 ef 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00      .
000000a0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  2f 2f 2e 2e 2f 52 65 61  6c 46 69 74 2f 62 6c 65                  //../RealFit/ble

When reading the ID back

00000000: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
00000020: 00 00 00 00 00 00 00 00  49 4d 62 61 63 6b 67 72  6f 75 6e 64 2d 63 6f 6d  70 72 65 73 73 65 64 2e          IMbackground-compressed.
00000040: 6a 70 67 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  jpg
00000060: 00 00 00 00 00 00 00 00  00 00 00 00 20 01 00 00  00 00 00 00 00 00 00 00  00 20 00 00 00 00 00 00               .
00000080: 00 00 00 00 ef 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00      .
000000a0: 00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  2f 2f 2e 2e 2f 52 65 61  6c 46 69 74 2f 62 6c 65                  //../RealFit/ble

Difference is in the ID struct are:

recalc (pos 0x78)
us (pos 0x70)
tag (pos 0x6b)

Looking at the comments just before these attributes these are cleares at read time...
Might be the issue...

Diff Detail

Repository

rB Blender

Branch

temp-T90593-undo-flushes-gpu-textures (branched from master)

Build Status

Buildable 16464
Build 16464: arc lint + arc unit

Event Timeline

Jeroen Bakker (jbakker) requested review of this revision.Aug 17 2021, 2:11 PM

Jeroen Bakker (jbakker) created this revision.

Harbormaster completed remote builds in B16464: Diff 40776.Aug 17 2021, 2:11 PM

Jeroen Bakker (jbakker) added a task: T90593: Undo takes several seconds just to move default cube within scene containing panorama if viewport shows materials.Aug 17 2021, 2:12 PM

Jeroen Bakker (jbakker) added a reviewer: Bastien Montagne (mont29).

Bastien Montagne (mont29) mentioned this in rBe648e388874a: Undo: Clear more ID runtime data on filewrite..Aug 19 2021, 3:10 PM

Thanks for this patch, but it had several issues hiding the actual cause.

Turns out, Image.lastused was not cleared/reset in write and read code, leading to the Image changed false detection.

Will fix that (along the other fixes in this patch).

Unfortunately this does not fully fix T90593, for some reason the first initial undo step still enforces full reload of the image, need to investigate further.

source/blender/blenkernel/intern/image.c
279	No need to store previous data here, the pointer that those callbacks get is already a temp copy generated by write code, precisely to allow tempering with the ID struct data itself easily.
290	This is a copy/paste mistake I guess? Obvious bug in any case, this needs to be called after everything has been cleared, and is already below in code. ;)
349–351	This is not the proper place to call this, as generic readfile calling code (aka `direct_link_id`/`direct_link_id_common`) has already restored a lot of those ID runtime data.