Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow aggregate component access with .[xyzrgb] #1049

Merged
merged 1 commit into from Aug 9, 2019

Conversation

@lgritz
Copy link
Member

commented Aug 6, 2019

Spatial geometry triples (point, vector, normal) now support A.x, A.y,
A.z as synonyms for A[0], A[1], A[2], respectively. It's not the
notation I prefer (obviously, or I would have done it 25 years ago),
but other people really dig it, so fine by me.

Colors now support A.r, A.g, A.b as synonyms for A[0], A[1], A[2],
respectively. To be honest, this worries me a little because I fear
we will regret it if we ever shift to supporting spectral rendering
(and the components will not necessarily represent red, green, and
blue, and may have arbitrary number). But again, people really seem to
like this, so fine, but don't come complaining to me in a few years
when your shaders all need rewriting for your new spectral renderer.

In both cases, it's for direct reference to lvalues (variables, including
references passed as arguments to functions):

val = P.x;   // yes
P.y = val;   // yes
function_with_float_param (P.z);   // yes

But you can't use this notations on arbitrary expressions:

val = function_returning_color().r;   // no
val = (P + N).y;                      // no, double ick

We are also NOT supporting "swizzling."

@fpsunflower

This comment has been minimized.

Copy link
Collaborator

commented Aug 6, 2019

LGTM!

@lgritz

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

I'm going to leave this up for a few days before merging, to give people a chance to comment, since it's rare that we make syntactic changes.

@MasterZap

This comment has been minimized.

Copy link

commented Aug 7, 2019

Thank you!!!

Question: Do you specifically care about color vs vector/point/normal type, or will color.x and vector.r also work (not that I particulary care, just curious)

Also, yeah, the spectre of spectral rendering... well.. it's not like every existing shader accessing the individual components wouldn't need to be rewritten anyway for that case (i.e. the problem is component access itself, not the syntax to acheive it), so that's kinda a red herring anyway, isn't it?

Especially since we have no color.numcomponents or similar in OSL (or do we?)

/Z

@lgritz

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2019

As I implemented it, .[rgb] for colors, .[xyz] for point, vector, normal.

We don't have a numcomponents, but it could be added.

@lgritz

This comment has been minimized.

Copy link
Member Author

commented Aug 9, 2019

Additional detail: this is 100% forward and back compatible. It's strictly an oslc-side change, so a shader that says "C.r" and is compiled with a new enough oslc to support that syntax can be executed just fine with an older ShadingSystem. Underneath, C.r is just a synonym for C[0].

Allow aggregate component access with .[xyzrgb]
Spatial geometry triples (point, vector, normal) now support A.x, A.y,
A.z as synonyms for A[0], A[1], A[2], respectively. It's not the
notation I prefer (obviously, or I would have done it 25 years ago),
but other people really dig it, so fine by me.

Colors now support A.r, A.g, A.b as synonyms for A[0], A[1], A[2],
respectively.  To be honest, this worries me a little because I fear
we will regret it if we ever shift to supporting spectral rendering
(and the components will not necessarily represent red, green, and
blue, and may have arbitrary number). But again, people really seem to
like this, so fine, but don't come complaining to me in a few years
when your shaders all need rewriting for your new spectral renderer.

In both cases, it's for direct reference to lvalues (variables, including
references passed as arguments to functions):

    val = P.x;   // yes
    P.y = val;   // yes
    function_with_float_param (P.z);   // yes

But you can't use this notations on arbitrary expressions:

    val = function_returning_color().r;   // no
    val = (P + N).y;                      // no, double ick

We are also NOT supporting "swizzling."

Note that this is strictly an oslc-side change, underneath C.r compiles
to the same ops as C[0] always did, so newly compiled shader using this
new syntax can be executed with old renderers/shading engines.

@lgritz lgritz force-pushed the lgritz:lg-component branch from 0c60997 to 7605040 Aug 9, 2019

@lgritz lgritz merged commit b6400e2 into imageworks:master Aug 9, 2019

1 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@lgritz lgritz deleted the lgritz:lg-component branch Aug 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.