osltoy - Feature Request: autodetect output color variable

[silvialpz]

Description

Fixes #2021

How it works:

• Getting all of the output color variables from OSL Query in build_shader_group() in order to set the first one as the default renderer_outputs in OSLToyRenderer.
• Creating Radio Buttons for output color param rows in the parameter layout panel.
• Setting up a getter function so that the user selected output can be used in OSLToyRenderer
• The selected output will be displayed when you hit recompile.

osltoy-auto-detect-output-variable.mov

Tests

I was working with a simple OSL file with two color output variables to test that the user could select the desired output with radio buttons.

surface test_noise (
    float scale = 10
        [[ string help = "Scaling factor for noise frequency" ]],
    output color rgb = 0
        [[ string help = "Perlin noise in color" ]],
    output color bw = 0
        [[ string help = "Perlin noise in black and white" ]]
    

)
{
    point uvw = point(u * scale, v * scale, 0);

    float n = noise("perlin", uvw);

    bw = color(n, n, n);
    rgb = color(noise("perlin", uvw),
             noise("perlin", uvw + 10),
             noise("perlin", uvw + 20));
}

Checklist:

• I have read the contribution guidelines.
• I have updated the documentation, if applicable.
• I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
• My code follows the prevailing code style of this project. If I haven't
  already run clang-format v17 before submitting, I definitely will look at
  the CI test that runs clang-format and fix anything that it highlights as
  being nonconforming.

[lgritz]

Great to see this land! I'm a little busy with releases for the next couple days, but will get to it right away after that.

Don't mind the failed CI for now, that is due to issues unrelated to your PR, and I'm fixing them.

[lgritz]

Can you please rebase on the current main and force push to trigger a new CI run to see what's working and what's failing?

[silvialpz]

It succeeded!

I thought seeing the latest commits on the main branch with a red ❌ meant it would fail on this too.

[lgritz]

I tested this and it does seem to work. Looks fine to me, though I made a couple of fairly minor cosmetic suggestions I'd like you to make, then we can merge.

[lgritz]

ustring by defaults constructs an empty string, so this can be more compactly expressed as

Suggested change

	// The currently selected output
	ustring m_selectedoutput = ustring(""); // Empty until set by user
	// The currently selected output
	ustring m_selectedoutput; // Empty until set by user

[lgritz]

Did you intend to remove this non-functional code?

[silvialpz]

Yes, oops 😬

[lgritz]

Let's break the comment onto a separate line in order to fix the awkward formatting here. And maybe also put this new item after m_framebuffer, intead of at the beginning? (Sorry, I can't make a diff for that through this interface; seems like GH will only let me include lines you actually changed in the diff, not surrounding code.)

Suggested change

	std::function<ustring()>
	m_get_selected_output; // Store the getter function that gets selection from app
	// Store the getter function that gets selection from app
	std::function<ustring()> m_get_selected_output;

[silvialpz]

okay done ✅

[lgritz]

Please note that the two Windows failures are not your fault. They are a GH thing, and I fixed them in a PR I merged that is not yet incorporated in your patch, but will be once you merge. Or you could rebase on top of current main to pick up those changes.

I recommend against ever doing "git merge", and instead think it's cleaner and easier to understand the history if you "git rebase main" and then force-push your branch. That tends to make a linear history that's easy to follow, rather than multiple branching merges.

[lgritz]

Aside to whoever is listening (not the fault of your PR, just something I noticed):

I noticed both in my testing of this, and Sylvia's screen shot, that at least on Mac, the color scheme is making the currently-edited line highlight color fully obscure the text in the code. Maybe it's been like that for a while (certainly not related to this PR), but I don't remember it being like that before. Maybe it's a recent change in the MacOS side, or the result of dark mode in our OS preferences? I'm not sure how to fix, but it's pretty annoying, so if anybody else knows how to fix that, please give us some tips!

[silvialpz]

I recommend against ever doing "git merge", and instead think it's cleaner and easier to understand the history if you "git rebase main" and then force-push your branch. That tends to make a linear history that's easy to follow, rather than multiple branching merges.

I see what you mean now, I used the the "sync fork" button on GitHub twice before but this does look nicer on the graph. Thanks!

[silvialpz]

Aside to whoever is listening ... the color scheme is making the currently-edited line highlight color fully obscure the text in the code.

I'm listening, I'll fix it

[lgritz]

LGTM