Page MenuHome
Differential D15265

RenderReport: Add option to add platform specific overrides.
ClosedPublic
Actions

Authored by Jeroen Bakker (jbakker) on Jun 22 2022, 9:56 AM.

Details

Reviewers
Brecht Van Lommel (brecht)
Clément Foucault (fclem)
Maniphest Tasks
T99046: Regression tests: Platform specific reference images
Commits
rB3393b7137e24: RenderReport: Add option to add platform specific overrides.
Summary

Reference images in the reference_override_dir will be chosen before
images in reference_dir. This allows platform specific reference
images, with a common base.

Ignored when set to None. The caller is responsible
of setting the reference override dir as the unit test is more aware
what the definition of a platform is.

Patch adds gpu.platform.device_type_get function to get the device
type that blender has detected.

Diff Detail

Repository
rB Blender
Branch
temp-T99046-platform-reference-images
Build Status
Buildable 23162
Build 23162: arc lint + arc unit

Event Timeline

Jeroen Bakker (jbakker) requested review of this revision.Jun 22 2022, 9:56 AM
Jeroen Bakker (jbakker) created this revision.
Harbormaster completed remote builds in B22626: Diff 52759.Jun 22 2022, 9:56 AM
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Jun 22 2022, 9:57 AM
Jeroen Bakker (jbakker) added reviewers: Brecht Van Lommel (brecht), Clément Foucault (fclem).
Jeroen Bakker (jbakker) added projects: EEVEE & Viewport, Restricted Project.
Jeroen Bakker (jbakker) updated this revision to Diff 52760.Jun 22 2022, 9:58 AM

Renamed to reference override dir

Harbormaster completed remote builds in B22627: Diff 52760.Jun 22 2022, 9:58 AM
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Jun 22 2022, 9:59 AM
Jeroen Bakker (jbakker) added a task: T99046: Regression tests: Platform specific reference images .Jun 22 2022, 10:03 AM
Jeroen Bakker (jbakker) updated this revision to Diff 52784.Jun 22 2022, 2:05 PM
Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)
  • Get GPU platform.
  • Added GPU info as script to solve initialization errors.
Harbormaster completed remote builds in B22637: Diff 52784.Jun 22 2022, 2:05 PM
Brecht Van Lommel (brecht) requested changes to this revision.Jun 22 2022, 4:37 PM
Brecht Van Lommel (brecht) added inline comments.
tests/python/gpu_info.py
2

It's not clear to me how the GPU backend gets initialized to be able to access the vendor.

5–7

Code style: + -> + .

tests/python/modules/render_report.py
88–92

Rename platform to override for consistency.

90

Should be os.path.exists(platform_img)?

This revision now requires changes to proceed.Jun 22 2022, 4:37 PM
Jeroen Bakker (jbakker) updated this revision to Diff 52795.Jun 22 2022, 5:13 PM
Jeroen Bakker (jbakker) marked 4 inline comments as done.

Added Python API for devce type.
Documented usage of gpu_info.
Removed references to platform.

Harbormaster completed remote builds in B22641: Diff 52795.Jun 22 2022, 5:13 PM
Jeroen Bakker (jbakker) added inline comments.Jun 22 2022, 5:14 PM
tests/python/gpu_info.py
2

The GPU backend is initialized when windows manager is initialized (hence I had to remove the --background parameter).
Also the --python-expr didn't work as expected.

Background rendering is working as draw manager detects and creates its own context. I found it to harmful to do a similar trick.

Will write a usage comment at the start of this script.

5

The vendor string is platform dependent. It would be better to use eGPUDeviceType, eGPUOSType and eGPUDriverType to reuse code.

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Jun 22 2022, 5:15 PM
Jeroen Bakker (jbakker) planned changes to this revision.Jul 22 2022, 10:08 PM
Jeroen Bakker (jbakker) marked an inline comment as not done.
Brecht Van Lommel (brecht) added a comment.Jul 25 2022, 6:34 PM

I missed that this patch got updated. I didn't spot issues in the current implementation, though not sure what kind of changes you have planned.

Popping up a window while running the tests is not ideal, not sure how much that would get in the way in practice. Possibly you could do a tiny workbench render to initialize the GPU in background mode.

Jeroen Bakker (jbakker) added a comment.Jul 25 2022, 10:25 PM

Found a bug where I missed a rename.

if os.path.exists(old_img):
Jeroen Bakker (jbakker) added a comment.Jul 26 2022, 2:29 PM

@Brecht Van Lommel (brecht) thanks for the tip. Using a tiny workbench render is a good idea.

Jeroen Bakker (jbakker) updated this revision to Diff 54066.Jul 26 2022, 2:31 PM
  • Fix issue using incorrect path.
  • Initialize GPU backend in background by rendering a 1x1 image.
Harbormaster completed remote builds in B23162: Diff 54066.Jul 26 2022, 2:31 PM
Brecht Van Lommel (brecht) accepted this revision.Jul 26 2022, 2:51 PM
This revision is now accepted and ready to land.Jul 26 2022, 2:51 PM
Jeroen Bakker (jbakker) marked an inline comment as done.Jul 28 2022, 8:41 AM
Closed by commit rB3393b7137e24: RenderReport: Add option to add platform specific overrides. (authored by Jeroen Bakker (jbakker)). · Explain WhyAug 1 2022, 10:57 AM
This revision was automatically updated to reflect the committed changes.
Jeroen Bakker (jbakker) added a commit: rB3393b7137e24: RenderReport: Add option to add platform specific overrides..

Revision Contents
Changeset List

PathSize
source/
blender/
python/
gpu/
32 lines
tests/
python/
27 lines
24 lines
modules/
17 lines

Diff 54066

View Options

source/blender/python/gpu/gpu_py_platform.c

Loading...
View Options

tests/python/eevee_render_tests.py

Loading...
View Options

tests/python/gpu_info.py

Loading...
View Options

tests/python/modules/render_report.py

Loading...