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

VRML/WRL mesh export #455

Closed
wants to merge 4 commits into from
Closed

VRML/WRL mesh export #455

wants to merge 4 commits into from

Conversation

nabijaczleweli
Copy link
Contributor

@nabijaczleweli nabijaczleweli commented Aug 9, 2019

As-is, any transparency is ignored, as it'd require emitting a Shape per transparency value (I think), so I'd like your opinion on the code as it currently stands before implementing that.

I'm not sure if you have any particular preferences regarding the std::vector<> usage for colour (and, later, transparency) buffering?

As for the material values, I present these four renders, identical, but for the material:

Just ambientIntensity 1:
ai1

Just ambientIntensity 0.3, as returned by SS.ambientIntensity:
ai.3

Material values copied from "Buzzer_12x9.5RM7.6.wrl" (which includes ambientIntensity 1):
a-ai1

Material values copied from "Buzzer_12x9.5RM7.6.wrl", but with ambientIntensity 0.3, as returned by SS.ambientIntensity:
a-ai.3

I can't speak for everyone, or for all renderers, but, to me, the last one both looks the best and truest to what I saw in SolveSpace, so that's what I've gone with.

… KiCad's Buzzer_12x9.5RM7.6.wrl

Colours are not deduplicated, transparency/alpha is unhandled
Declutters and seems to be parsed correctly
@whitequark
Copy link
Contributor

Thank you for the PR. The only part that I think might be an issue is the "magic values"; although I acknowledge that getting good results in KiCAD is an important goal, I think it is also important to not have hardcoded values that favor one specific workflow to detriment of other ones.

Have you compared the rendering results in SolveSpace and KiCAD? If you use SS.ambientIntensity do they match? If they do, then it sounds like for your workflow, you should set SS.ambientIntensity to match KiCAD in the first place.

@whitequark
Copy link
Contributor

Actually, now that I look closer, I'm not sure if it makes sense to use ambientIntensity since in SolveSpace it is combined with the user-defined light sources, and in VRML this isn't applied (and can't be). It's not quite clear what's the best path forward here.

@whitequark
Copy link
Contributor

According to this, diffuseColor should be ignored with colored textures; what about colored triangles? Does it come into play here at all?

@nabijaczleweli
Copy link
Contributor Author

nabijaczleweli commented Aug 9, 2019

Hmm, when I update ambientIntensity = 0.3; // no setting for that yet to be 1, instead, the model looks, well, uniformly fully lit, as expected (and haven't found a setting in KiCad).

But, I played around with the material a bit more, and reduced it to pretty much just diffuseColor making or breaking the rendering (ambientIntensity being present or not does change the render, but to a very small degree in this case). So I searched for "diffuse" in the source and came across GL calls in rendergl1.cpp, so I plugged in SS.ambientIntensity in there, too, and it looked equivalently good.

Which, as I understand it (that is: poorly), and correct me if I'm wrong, is to be expected: the exported model specifies how it is supposed to look/behave, or under which conditions it looks "neutral" with its material spec, right? So, if we specify the material that we use within SolveSpace, in this case lighting conditions, was created and looks neutral under [our set of conditions], the renderer that uses the export knows to apply those instead of or on top of its defaults/environment?

Anyway, this is the material I arrived at, and a screenshot demonstrating it:

diffuseColor 0.3 0.3 0.3
ambientIntensity 0.3
transparency 0.0

ai.3+dc.3

The same but without ambientIntensity:

-ai+dc.3

Without diffuseColor:

ai.3-dc

With neither:

-ai-dc

The colour of this package's body is set to 0.07 across the board in SolveSpace, so that's not a great colour

Granted, diffuseColor might be slightly different de iure (like idk, depend on the other two lights), but I know absolute fuck-all about 3D, so I used a value that I had on hand (and which turned out to look good). I'd be happy to test with other software, but I neither have nor, really, know of any

@whitequark
Copy link
Contributor

Merged as 837628e.

@whitequark whitequark closed this Aug 9, 2019
@nabijaczleweli
Copy link
Contributor Author

nabijaczleweli commented Aug 9, 2019

I was also gonna add proper transparency handling since you didn't have anything against my caching scheme, but I guess I'll open a second PR?

@nabijaczleweli nabijaczleweli deleted the vrml_export branch August 9, 2019 21:37
@whitequark
Copy link
Contributor

Yep, that works!

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

Successfully merging this pull request may close these issues.

None yet

2 participants