Details

Reviewers

Maniphest Tasks

T99998: Image: Display GPU layout in `uiTemplateImageInfo`

Commits

rBea70687dd5b9: Image: Display GPU layout in `uiTemplateImageInfo`

Summary

Add IMB_gpu_get_texture_format and GPU_texture_format_description to
retrieve and 'stringify' an eGPUTextureFormat. These are then used in the
image info panel used in several areas across blender.

New Information:

Diff Detail

Repository

rB Blender

Branch

T99998-gpu-texture-ui (branched from master)

Build Status

Buildable 23226
Build 23226: arc lint + arc unit

Event Timeline

Angus Stanton (abstanton) requested review of this revision.Jul 29 2022, 2:09 PM

Angus Stanton (abstanton) created this revision.

Harbormaster completed remote builds in B23204: Diff 54170.Jul 29 2022, 2:09 PM

Angus Stanton (abstanton) added a reviewer: Jeroen Bakker (jbakker).Jul 29 2022, 2:09 PM

Angus Stanton (abstanton) retitled this revision from T99998: Display GPU layout in `uiTemplateImageInfo` to Image: Display GPU layout in `uiTemplateImageInfo`.

Thanks for the patch. I did a first pass, haven't tested it on all platforms yet, but will do that later.

intern/ghost/CMakeLists.txt
10	Not sure why this is needed.
133	Think these formatting changes should not be added in this patch. Perhaps disable auto formatting for cmakelist files as I am not aware of a ruleset we use at blender.
source/blender/imbuf/IMB_imbuf.h
936	add `const` keyword to reduce compilation warnings on linux.
source/blender/imbuf/intern/util_gpu.c
304	When using this interface we should perhaps move it to the gpu module now it isn't ImBuf specific.
307	no need for (..), please remove when not needed
308	I would remove the spaces and keep close to the technical key. So GPU_RGBA8UI would become RGBA8UI
381	Remove commented out code here.
388	Same here.
438	Same here
455	DEPTH32F would be concise.

This revision now requires changes to proceed.Aug 1 2022, 11:11 AM

Merge branch 'master' into T99998-gpu-texture-ui
Address comments

Harbormaster completed remote builds in B23226: Diff 54261.Aug 1 2022, 5:34 PM

Angus Stanton (abstanton) added inline comments.Aug 1 2022, 5:36 PM

intern/ghost/CMakeLists.txt
10	It is needed to include GPU_texture.h , or at least that's how I fixed an error, maybe it wasn't the best way to do it?
133	Sure, I'll try and disable the auto formatting for CMake files.
source/blender/imbuf/intern/util_gpu.c
304	As in, move to the GPU_texture.h header?

Move IMB_gpu_get_texture_format_description to GPU

Harbormaster completed remote builds in B23227: Diff 54262.Aug 1 2022, 5:51 PM

Almost there! some minor consistency changes needed in the descriptions.
Also we should update the description of this change it still refers to the old function name.
Perhaps also add a note in the task description why and where a user could see this information.

intern/ghost/CMakeLists.txt
10	Yes, dependency is needed as imbuf includes gpu which includes blenlib.
source/blender/gpu/intern/gpu_texture.cc
684 ↗	(On Diff #54262)	Remove space
720 ↗	(On Diff #54262)	Add semi-column with previous line
723 ↗	(On Diff #54262)	Remove space.