blender/blender-test-data
0
0
Fork 7
Code Pull Requests 6 Activity

Cycles: Add render tests for texture nodes #16

Open
Alaska wants to merge 15 commits from add-texture-tests into main
pull from: add-texture-tests
merge into: blender:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 5 Commits 15 Files Changed 76 +228
Alaska commented 2024-10-11 05:15:27 +02:00
Member
Copy Link

Adds render tests for most texture nodes in Blender, testing multiple
configurations per nodes, and including some previously problematic
test causes to ensure they don't appear in the future.

Some nodes (E.g. IES) aren't tested as they're better suited for
another category.

Ref blender/blender#123012

Adds render tests for most texture nodes in Blender, testing multiple configurations per nodes, and including some previously problematic test causes to ensure they don't appear in the future. Some nodes (E.g. IES) aren't tested as they're better suited for another category. Ref blender/blender#123012
Alaska added 15 commits 2024-10-11 05:15:29 +02:00 
- Added test for "no mapping white nosie"
- Added a test for a precision issue in the musgrave texture
If there are two materials, and the nodes are the same, but the outputs are different between them
then use group nodes to keep the nodes in sync.

In the process of making this change, it was found that some materials weren't kept in sync

I also adjusted the anisotropy in the Garbor noise test. 12598 talks about a rotational bias
in garbor noise anisotropy, so increasing the ansiotropy should make it more visible in the
test file, and thus make it easier to fail when the issue is fixed.
Increase the frequency of the low frequency test from 0.02 to 0.3
blender/blender@ce89d7949c
adjusted the appearance and thus I needed to increase the value
to make it visible.

Add more tests for 2D Garbor for anisotropy, triggered by:
blender/blender@075bdbcd63
Alaska requested review from Sergey Sharybin 2024-10-11 05:15:47 +02:00
Alaska requested review from Lukas Stockner 2024-10-11 05:15:48 +02:00
Alaska requested review from Weizhen Huang 2024-10-11 05:15:48 +02:00
Alaska commented 2024-10-11 05:19:38 +02:00
Author
Member
Copy Link

This PR was originally !6

However to test this PR on the build bot (a step I plan to do before committing), I need the branch to be in blender/blender-test-data, so I had to create a new PR.

This PR was originally !6 However to test this PR on the build bot (a step I plan to do before committing), I need the branch to be in `blender/blender-test-data`, so I had to create a new PR.
Sergey Sharybin commented 2024-10-11 11:34:08 +02:00
Owner
Copy Link

@Alaska I am a bit confused. For a new branch on blender-tests-data you can just commit directly to a branch, and create PR against blender.git. This PR is against main, so if we land it kind of defeats the purpose of having it in a branch first?

As for adding texture tests - it is something i am all up for, so it is more about figuring out logistics of how to get them into the repos.

@Alaska I am a bit confused. For a new branch on `blender-tests-data` you can just commit directly to a branch, and create PR against `blender.git`. This PR is against `main`, so if we land it kind of defeats the purpose of having it in a branch first? As for adding texture tests - it is something i am all up for, so it is more about figuring out logistics of how to get them into the repos.
Alaska commented 2024-10-11 11:55:47 +02:00
Author
Member
Copy Link

My understanding was this:

  • To run new tests on the buildbot, you must:
    • Create a branch on blender/blender-tests-data not YOUR_NAME/blender-tests-data with the new tests
    • Then you must create a PR against blender/blender that references the tests (That's here: blender/blender#124574)
    • Then you can run the build bot command.

I opened this as a PR with reviewers because it's a larger collection of new tests, so I'd like them to be reviewed.

My understanding was this: - To run new tests on the buildbot, you must: - Create a branch on `blender/blender-tests-data` not `YOUR_NAME/blender-tests-data` with the new tests - Then you must create a PR against `blender/blender` that references the tests (That's here: https://projects.blender.org/blender/blender/pulls/124574) - Then you can run the build bot command. I opened this as a PR with reviewers because it's a larger collection of new tests, so I'd like them to be reviewed.
Sergey Sharybin approved these changes 2024-10-11 12:23:19 +02:00
Sergey Sharybin left a comment
Owner
Copy Link

Your understanding is fully correct!
I think I understand now: i've missed the fact that the source branch is in the blender/blender-tests-data, and not in the fork. Makes it possible to easily test locally and poke buildbot in blender/blender#124574.

Form quick looking into the PR it seems all good! I've mainly:

  • looked at the reference images
  • applied the changes locally, and run compile + tests. Surely there is time "penalty" of 9 seconds, but I think it worth for having an increased confidence of textures rendering correctly after possible refactors and such.

I've also poked CI/CD on the PR against blender.git to ensure all other platforms are happy.

Your understanding is fully correct! I think I understand now: i've missed the fact that the source branch is in the `blender/blender-tests-data`, and not in the fork. Makes it possible to easily test locally and poke buildbot in blender/blender#124574. Form quick looking into the PR it seems all good! I've mainly: - looked at the reference images - applied the changes locally, and run compile + tests. Surely there is time "penalty" of 9 seconds, but I think it worth for having an increased confidence of textures rendering correctly after possible refactors and such. I've also poked CI/CD on the PR against `blender.git` to ensure all other platforms are happy.
Alaska commented 2024-10-11 13:28:26 +02:00
Author
Member
Copy Link

My main concern would be that I'm testing too many configurations of some things.

For example I test a lot of configurations of the wave texture, many of which probably aren't needed.

Another thing that I talked about a while ago but should probably re-mention here:

I created most of the tests in Blender 3.6, then created a few additional tests in Blender 4.1 and 4.2 for features/nodes that weren't in 3.6. Some of the nodes in 3.6 no longer exist. For example the Musgrave texture, but the versioning code is tested with tests like musgrave_multifractal_36 (A file created and saved in 3.6 using the musgrave texture).

I verified locally that the 3.6 specific test files (like musgrave) did get correctly converted to 4.2.

My main concern would be that I'm testing too many configurations of some things. For example I test a lot of configurations of the wave texture, many of which probably aren't needed. Another thing that I talked about a while ago but should probably re-mention here: I created most of the tests in Blender 3.6, then created a few additional tests in Blender 4.1 and 4.2 for features/nodes that weren't in 3.6. Some of the nodes in 3.6 no longer exist. For example the Musgrave texture, but the versioning code is tested with tests like `musgrave_multifractal_36` (A file created and saved in 3.6 using the musgrave texture). I verified locally that the 3.6 specific test files (like musgrave) did get correctly converted to 4.2.
Sergey Sharybin commented 2024-10-11 14:46:05 +02:00
Owner
Copy Link

My main concern would be that I'm testing too many configurations of some things.

This perfectly follows the blackbox testing method. Perhaps some of the permutations you test can be avoided due to them falling into the same code path, but it is not an assumption you can make from a blackbox perspective.

> My main concern would be that I'm testing too many configurations of some things. This perfectly follows the blackbox testing method. Perhaps some of the permutations you test can be avoided due to them falling into the same code path, but it is not an assumption you can make from a blackbox perspective.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin add-texture-tests:add-texture-tests
git checkout add-texture-tests
Sign in to join this conversation.
Reviewers
No reviewers
Lukas Stockner
Weizhen Huang
Sergey Sharybin
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender-test-data#16
No description provided.
Delete Branch "add-texture-tests"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?