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

Attribute directive usage #34

Closed
shadeops opened this issue Sep 6, 2020 · 8 comments
Closed

Attribute directive usage #34

shadeops opened this issue Sep 6, 2020 · 8 comments

Comments

@shadeops
Copy link
Contributor

shadeops commented Sep 6, 2020

A Question:

As far as I can tell, being able to specify Material overrides on Shapes is no longer supported.
If this is correct, how come? Due to GPU implementation? Simplification of code? (I'm not advocating for the return of this functionality I'm simply curious.)

Background:

In pbrt-v3 you could do something along the lines of -

# From https://www.pbrt.org/fileformat-v3.html#materials
Material "matte" "rgb Kd" [ 1 0 0 ]
Shape "sphere" "float radius" [ .5 ] "rgb Kd" [ 0 1 0 ]  

In pbrt-v4 this results in a unused parameter error.

A Follow-up:

Assuming this functionality is no longer supported, then the following code in parsedscene.cpp should probably be changed.

https://github.com/mmp/pbrt-v4/blob/08f863d3/src/pbrt/parsedscene.cpp#L1776

void FormattingScene::Shape(
        ...
        dict.RenameParameter("Kd", "reflectance");

Which renames the "Kd" parameter found on Shapes to "reflectance". This would still result in an error in pbrt-v4, so it should probably be replaced with a dict.RemoveTexture("Kd"). Or perhaps more appropriate a straight up error instructing the user to update their scenes.

@mmp
Copy link
Owner

mmp commented Sep 10, 2020

Yep, that functionality is gone in pbrt-v4. It was basically a combination of it adding a fair amount of code complexity in return for not being used very much. (Never at all in the scenes I've got..)

(Happy to consider adding it back if you or anyone else cares!)

@shadeops
Copy link
Contributor Author

I cared and thought it was a useful feature because -

  • PrimVars, (having look related parameters be overridden by geometry), is very much a bread and butter technique in VFX production rendering. While pbrt is of course not meant to be a VFX production renderer, I do believe there is value in introducing the concept and workflow to students. Whether pbrt is the appropriate place to do so given all the other topics that need to be covered is a good question.

Some of the more tangible benefits comes when dealing with very large number of Shapes, for example: hair, grass, leaves, or spheres in a gumball machine. In cases like these overrides allow for-

  • Reduced scene size (and Material/Texture instances), as you don't have to replicate the entire shading graph in many cases.

    • Counter point: While having 10,000s of extra Material instances does use more resources, that overhead is eclipsed by all the mesh data.
  • Faster scene export. For example if you are overriding some parameters on the Material that don't need a texture graph, the export can be significantly faster than having to generate the graph.

    • Counter point: That's not a pbrt problem. ;)

That said, on the exporter side allowing for the overrides ended up being the most convoluted aspect of the design and part of me is eager to simplify (hence the question above). =]

@mmp
Copy link
Owner

mmp commented Nov 9, 2020

On PrimVars: yeah, it would be nice to have more about that. Though FWIW pbrt never even supported arbitrary per-vertex attributes that were interpolated before being used during shading--it was just uniforms, textures or the somewhat oddball BilerpTexture, which is gone in v4. The overrides it did support were implemented by taking care of them during parsing and creating a whole new material instance rather than having a more flexible material interface that allowed overrides and then figured out the right thing at runtime, which probably would have been a better example to set.

Now, there is one piece of good news: there's a new Attribute directive that obeys the graphics state. When an attribute of a particular type is set, then it is included in the parameters used to create it. So,

Material "diffuse"
Attribute "material" "rgb reflectance" [1 0 0]
Shape "sphere"
Attribute "material "rgb reflectance" [0 1 0 ]
Translate 1 0 0
Shape "sphere"

Should get a lot of the way there. (I totally forgot about this--it's new. It's also not well-tested, so bug reports welcome. :-) )

@shadeops
Copy link
Contributor Author

Oh wow despite reading through parsedscene.cpp multiple times that never clicked.
Thanks for flagging! I'll give it a try. 👍

@shadeops
Copy link
Contributor Author

shadeops commented Dec 7, 2020

Hey @mmp ,
I was giving the Attribute directive a try and got the following results:

The following works -

Attribute "material" "rgb reflectance" [1 0 0]
Material "diffuse
Shape "sphere"

Attribute "material "rgb reflectance" [0 1 0 ]
Material "diffuse"
Translate 1 0 0
Shape "sphere"

However the following does not, which is at odds with your example above -

Material "diffuse"
Attribute "material" "rgb reflectance" [1 0 0]
Shape "sphere"
Attribute "material "rgb reflectance" [0 1 0 ]
Translate 1 0 0
Shape "sphere"

A complete example:
Expectation: Left spheres should be blue, right spheres should be red.
attributes

Film "rgb" 
    "integer xresolution" [ 400 ]
    "integer yresolution" [ 300 ]
    "string filename" [ "test.exr" ]

PixelFilter "gaussian" 
    "float yradius" [ 2 ]
    "float xradius" [ 2 ]

Sampler "pmj02bn"
    "integer pixelsamples" [ 256 ]

Integrator "volpath"
    "integer maxdepth" [ 5 ]

Accelerator "bvh"

LookAt  0 20 -50
        0 5 0
        0 1 0

Camera "perspective"
        "float fov" [ 45 ]
        "float screenwindow" [ -1 1 -0.75 0.75 ]

WorldBegin

LightSource "infinite" "float scale" [ 1 ]

MakeNamedMaterial "matte" "string type" "diffuse"
    "rgb reflectance" [ 0.5 0.5 0.5 ]

AttributeBegin
    NamedMaterial "matte"
    Rotate 90 1 0 0 
    Shape "disk" "float radius" [ 100 ]
AttributeEnd

Texture "checkerboard" "spectrum" "checkerboard"
    "float uscale" [ 10 ]
    "float vscale" [ 10 ]

Material "diffuse"

# Lower Spheres
# Attributes after the Material directive.

# Blue
AttributeBegin
    Attribute "material" "rgb reflectance" [ 0.1 0.1 0.8 ]
    Translate -10 4 0
    Shape "sphere" "float radius" [ 3 ]
AttributeEnd

# Red
AttributeBegin
    Attribute "material" "rgb reflectance" [ 0.8 0.1 0.1 ]
    Translate 10 4 0
    Shape "sphere" "float radius" [ 3 ]
AttributeEnd


# Upper Spheres
# Attributes before the Material directive

# Blue
AttributeBegin
    Attribute "material" "rgb reflectance" [ 0.1 0.1 0.8 ]
    Material "diffuse"
    Translate -10 12 0
    Shape "sphere" "float radius" [ 3 ]
AttributeEnd

# Red
AttributeBegin
    Attribute "material" "rgb reflectance" [ 0.8 0.1 0.1 ]
    Material "diffuse"
    Translate 10 12 0
    Shape "sphere" "float radius" [ 3 ]
AttributeEnd

@shadeops shadeops changed the title Material overrides on Shapes in pbrt-v4 Attribute directive usage Sep 16, 2021
@shadeops
Copy link
Contributor Author

Hello,

I renamed this ticket to be about how the Attribute directive works, just so I understand the intended use case. (With the original material overrides on shapes question being way out of scope for pbrt-v4. )

@mmp
Copy link
Owner

mmp commented Sep 22, 2021

Yep, good question. The intended use case is, generally speaking, being able to set default parameters for things (shapes, textures, etc.). e.g.:

Attribute "shape" "float width" 1.5
# all subsequent "curve" shapes now have a default width of 1.5 if a width is not directly specified in their parameters

This is mostly intended as a convenience for people extending the system (e.g. student projects): "I'd like to indicate that all these shapes have a certain property--how can I do that?"

@shadeops
Copy link
Contributor Author

Ah cool, thanks for the explanation. Good feature for debugging too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants