PixarAnimationStudios / OpenSubdiv

An Open-Source subdivision surface library.
graphics.pixar.com/opensubdiv
Other
2.89k stars 561 forks source link

Color indexing in far tutorial_2_2 using wrong limit ? #1241

Closed nyue closed 2 years ago

nyue commented 2 years ago

I believe we should be using ncolors rather than nuvs

https://github.com/PixarAnimationStudios/OpenSubdiv/blob/release/tutorials/far/tutorial_2_2/far_tutorial_2_2.cpp#L336

This tutorial example may also benefit from writing out to PLY as it supports point colors.

If there is agreement, I can take a stab at the above.

Cheers

jilliene commented 2 years ago

Filed as internal issue #OSD-373

barfowl commented 2 years ago

I was doing some other minor work in the Far tutorials and decided to include the fix to this bug. Thanks for pointing it out.

Regarding changing the output of this tutorial to PLY, I'm personally not in favor of generating another output format. Despite its limitations, Obj files can be read into, written from and viewed with the OpenSubdiv tutorials and examples. So I like the idea of being able to use a tool within the repository to validate something another one generates (there are a few exceptions).

One thing we might want to do with this tutorial is eliminate its use of color. This tutorial is not unique in generating color (2.1 also uses it), but it is unique in that it uses more than one channel of face-varying data. We could simply replace color with another set of UVs, e.g. one set packed into a single atlas and another with different UV sets tiled for rendering, and simply use a runtime option to generate one or the other to confirm.

I mocked up something like that a while ago and may still have something around to use if we decided to go that route...

davidgyu commented 2 years ago

Fixed by #1251

Thanks for catching this! Agree, we'll defer addition of alternative output formats for now.