Fix: No image GPU texture update for different requests #105547

Merged
Omar Emara merged 3 commits from OmarEmaraDev/blender:fix-no-update-for-multi-pass-gpu-textures into main 2023-03-21 14:57:24 +01:00
3 changed files with 35 additions and 1 deletions
Showing only changes of commit 09138963cd - Show all commits

View File

@ -493,11 +493,28 @@ struct ImBuf *BKE_image_get_first_ibuf(struct Image *image);
*/
struct GPUTexture *BKE_image_create_gpu_texture_from_ibuf(struct Image *image, struct ImBuf *ibuf);
/**
* Ensure that the cached GPU texture inside the image matches the pass, layer, and view of the
* given image user, if not, invalidate the cache such that the next call to the GPU texture
* retrieval functions such as BKE_image_get_gpu_texture updates the cache with an image that
* matches the give image user.
*
* This is provided as a separate function and not implemented as part of the GPU texture retrieval
* functions because the current cache system only allows a single pass, layer, and stereo view to
* be cached, so possible frequent invalidations of the cache can have performance implications,
* and making invalidation explicit by calling this function will help make that clear and pave the
* way for a more complete cache system in the future.
*/
void BKE_image_ensure_gpu_texture(struct Image *image, struct ImageUser *iuser);
/**
* Get the #GPUTexture for a given `Image`.
*
* `iuser` and `ibuf` are mutual exclusive parameters. The caller can pass the `ibuf` when already
* available. It is also required when requesting the #GPUTexture for a render result.
*
* Call BKE_image_ensure_gpu_texture first if the requested layer, pass, and view might differ
OmarEmaraDev marked this conversation as resolved Outdated

This is more the exception. First we should try to fix it in the RNA/Operator. If this isn't possible we can use this function.

This is more the exception. First we should try to fix it in the RNA/Operator. If this isn't possible we can use this function.

Can you clarify? Why operators are we talking about?

Can you clarify? Why operators are we talking about?

Any operator that alters an image is responsible to invalidate the area of the image that is effected. For example reload, resize, flip etc. Same for any UI change done via the RNA layer. Change type, change render pass etc.

Only mentioning BKE_image_ensure_gpu_texture without describing the desired implementation will perhaps point developers in to use the workaround in places where it isn't required.

The issue we are solving is temporary and should eventually be supported differently. Viewport compositor should be able to construct a multipass image. and don't rely on the cached gpu texture. This is because the life-time cannot be guaranteed for the whole draw pipeline. You might want to combine multiple renderpasses of the same image. This is out of scope for this fix and release.

Does this make sense?

Any operator that alters an image is responsible to invalidate the area of the image that is effected. For example reload, resize, flip etc. Same for any UI change done via the RNA layer. Change type, change render pass etc. Only mentioning `BKE_image_ensure_gpu_texture` without describing the desired implementation will perhaps point developers in to use the workaround in places where it isn't required. The issue we are solving is temporary and should eventually be supported differently. Viewport compositor should be able to construct a multipass image. and don't rely on the cached gpu texture. This is because the life-time cannot be guaranteed for the whole draw pipeline. You might want to combine multiple renderpasses of the same image. This is out of scope for this fix and release. Does this make sense?
Review

Yes, makes sense. I have updated the description to describe that.

Yes, makes sense. I have updated the description to describe that.
* across calls.
*/
struct GPUTexture *BKE_image_get_gpu_texture(struct Image *image,
struct ImageUser *iuser,

View File

@ -336,6 +336,20 @@ static void image_gpu_texture_try_partial_update(Image *image, ImageUser *iuser)
}
}
void BKE_image_ensure_gpu_texture(Image *image, ImageUser *image_user)
{
if (!image) {
return;
}
/* Note that the image can cache both sterio views, so we only invalidate the cache if the view
* index is more than 2. */
if (image->gpu_pass != image_user->pass || image->gpu_layer != image_user->layer ||
(image->gpu_view != image_user->multi_index && image_user->multi_index >= 2)) {
BKE_image_partial_update_mark_full_update(image);
}
}
static GPUTexture *image_get_gpu_texture(Image *ima,
ImageUser *iuser,
ImBuf *ibuf,
@ -365,7 +379,9 @@ static GPUTexture *image_get_gpu_texture(Image *ima,
ima->gpu_pass = requested_pass;
ima->gpu_layer = requested_layer;
ima->gpu_view = requested_view;
BKE_image_partial_update_mark_full_update(ima);
/* The cache should be inavlidated here, but it is inmtentionally isn't due to possible
OmarEmaraDev marked this conversation as resolved Outdated

Small typo. A life saver could be to use "Code Spell Checker" when using VSCode.

Small typo. A life saver could be to use "Code Spell Checker" when using VSCode.
* performance implications, see the BKE_image_ensure_gpu_texture function for more
* information. */
}
#undef GPU_FLAGS_TO_CHECK

View File

@ -516,6 +516,7 @@ class ImageOperation : public NodeOperation {
}
ImageUser image_user = compute_image_user_for_output(identifier);
BKE_image_ensure_gpu_texture(get_image(), &image_user);
GPUTexture *image_texture = BKE_image_get_gpu_texture(get_image(), &image_user, nullptr);
const int2 size = int2(GPU_texture_width(image_texture), GPU_texture_height(image_texture));