Skip to content

ofBox 3dPrimitive winding order inconsistent #2676

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

Open
tgfrerer opened this issue Nov 11, 2013 · 15 comments
Open

ofBox 3dPrimitive winding order inconsistent #2676

tgfrerer opened this issue Nov 11, 2013 · 15 comments

Comments

@tgfrerer
Copy link
Member

All primitives but ofBoxPrimitive appear to be in CW winding order, resulting in front faces to be culled instead of backfaces, when glCullFace is enabled.

test case:

https://gist.github.com/tgfrerer/7417271

test case render

screenshot 2013-11-11 17 46 57

related issues

We have dealt with winding isssues in the past, with #2263 being the most recent.
There is a vintage open issue addressing this: #745

thoughts

I could fix/unify this, but i think it's worth discussing which winding order we adopt as standard. I'd personally be for going with the OpenGL standard of CCW for front-facing triangles, which would mean to bring everyting in line with ofBoxPrimitive. my main reason being that these kind of special cases could make oF a bit more difficult to learn / and code harder to port.

@arturoc
Copy link
Member

arturoc commented Nov 12, 2013

the only problem i see is with polygons we've always done CW but i guess it would be ok to change them too?

@tgfrerer
Copy link
Member Author

Yeah, really tricky, I'd be totally for it, although i fear it might glitch people's old projects. We'd have to be very explicit about this in the changelog. One alternative thing to consider would be to setup OpenGL using glFrontFace explicitly to:

glFrontFace(GL_CW); // default is: glFrontFace(GL_CCW);

But generally, I think it could be nice to clean the API to stick to OpenGL defaults/expectations, to keep with the principle of least surprise. Also, keeping to the standard would make it easier to port from other sources.

My concern with the glFrontFace-method would be that it would be an "invisible" fix for beginners, but a really hard to find/debug openFrameworks 'special case' for more advanced users.

I've googled to see whether the polygon CW winding might historically have something to do with cairo / svg and negative space / polygon fill rules, and it seems to me that svg uses CW winding: http://www.w3.org/TR/SVG/painting.html#FillRuleProperty
but then, most literature (and the default OpenGL render settings) go counter-clockwise for polygons, too. Hm...

@bilderbuchi
Copy link
Member

so, @tgfrerer, has this issue been closed by the merged PR #2267?

1 similar comment
@bilderbuchi
Copy link
Member

so, @tgfrerer, has this issue been closed by the merged PR #2267?

@tgfrerer
Copy link
Member Author

not yet, afaik, the case is still awaiting consensus. PR #2267 brought the icosphere in line to use (CW) like most other default primitives, but box still uses CCW, i believe. I can't run the test case just now, but it would give us a definite answer on the state of things…

@bilderbuchi
Copy link
Member

I don't think even the `IcoSpherePrimitive is working correctly. I'm on current master (815b2b7), and if I add

    ofPushMatrix();
    ofTranslate(20, -20);
    ofIcoSpherePrimitive(5,2).getMesh().draw();
    ofPopMatrix();

to your test case, I get

screenshot from 2014-01-21 16 35 05

which looks wrong to me.

@tgfrerer
Copy link
Member Author

Right, icosphere is CW, like sphere - and all but ofBox, which is wound CCW.

If I remember correctly, the PR you mentioned brought icosphere in line with the others(CW), so the render seems ok to me, but we still should decide, I think, whether we want all primitives CCW (like ofBox, and OpenGL default) or CW (like openFrameworks by tradition), just to at least be consistent.

Also important to notice that one can switch opengl to use CW for front faces by calling glFrontFace(GL_CW); or so before culling. It's just something that can trip people up.

Cheers

T

typed on toucg

On 21 Jan 2014, at 15:37, Christoph Buchner notifications@github.com wrote:

I don't think even the `IcoSpherePrimitive is working correctly. I'm on current master (815b2b7), and if I add

ofPushMatrix();
ofTranslate(20, -20);
ofIcoSpherePrimitive(5,2).getMesh().draw();
ofPopMatrix();

to your minimal example, I get

which looks wrong to me.


Reply to this email directly or view it on GitHub.

@ofTheo
Copy link
Member

ofTheo commented Mar 30, 2014

moving this to 0.8.2 and pinging @NickHardeman as he did most of the primitives stuff.
agree this is a tough one to change. switching the order to ccw could potentially break a lot of projects.

@bilderbuchi bilderbuchi modified the milestones: 0.8.3, 0.8.2 May 4, 2014
@bilderbuchi bilderbuchi modified the milestones: 0.8.4, 0.9.0 Aug 10, 2014
@bilderbuchi bilderbuchi modified the milestones: 0.9.0, 0.9.1 Sep 3, 2014
@kylemcdonald kylemcdonald modified the milestones: 0.9.3, 0.9.1, 0.9.2 Dec 6, 2014
@kylemcdonald kylemcdonald removed this from the 0.9.1 milestone Dec 6, 2014
@vade
Copy link
Contributor

vade commented Dec 8, 2014

FWIW, I think it best to stick to OpenGL defaults - this means that people new to programming, who benefit from OpenFrameworks libraries and wrappers, won't face issues when reading tutorials on OpenGL and trying to write their own drawing code, etc etc.

Leaving GL state clean and as untouched is helpful (and very hard to do), but easier the less you deviate from defaults.

Frankly, for an artists toolkit, I think backwards compatibility is less important than correctness and internal compatibility with built in intrinsics and functions. I for one have had issues where drawing geometry worked fine, but then drawing to an FBO didn't, because of different winding orders in fbo.draw and the geometry drawing. Things like this are killer time wasters for even the best of us, and for the uninitiated make it appear that OF is broken in some cases.

I know I bitch and moan a lot, but these small things add up, and fixing it sooner rather than waiting just saves you pain from doing it later, and having it more entrenched.

@vade
Copy link
Contributor

vade commented Dec 8, 2014

Maybe a suggestion - do ofxAddons have an API that queries what version of the OpenFrameworks API they are linking against?

Versioning could be helpful for ofxAddons.

New spec for add ons adds a function that returns an 'compatible version string' check. Existing add ons that don't have that function pointer you know are 'old' and make the old API assumptions.

New ofxAddons return a version string for the version of OF or the renderer they are built against. This allows you in the future begin a series of updates while being able to VERY VERBOSELY inform users and developers of rendering default changes, so code can be updated and people at least have some clue as to what is what, and why.

Just thinking aloud.

@memo
Copy link
Member

memo commented Dec 8, 2014

As discussed on this epic twitter thread https://twitter.com/memotv/status/542013046059978752 :) the deeper problem is the fact that OF by default uses left handed coordinates, like processing, unity, cinema4d, unreal, directx etc. Whereas opengl is right handed by default, like blender, maya etc. (actually opengl is RH only in 3D, as soon as it's clipped it switches to LH. so shaders operate in LH space).

To go from LH to RH or vice versa you need to flip an axis, and that reverses the winding order which flips the normals. This is why objects appear 'inside out' when you import them if they don't come from the same handed coordinate space. E.g. importing blender models in unity, or C4d models in blender etc. So you have to manually flip the normals (i.e. winding direction) after loading it. Unfortunately most 3d mesh files do not have the metadata to say which coordinate system it comes from (e.g. obj, ply, 3ds). They only store 3d data with no information to the handedness. So this flipping can't be done automatically. You need to load the model, see that it's screwed and flip it. OR if you know where the model is coming from you can flip on load. FBX is assumed to be RH, (but C4D exports LH FBX. Annoying bug. Such is life. Maybe it's fixed by now).

Having inconsistency with the winding order within OF is pretty terrible and should definitely be unified. To what it should be unified is a big question. LH (current OF default) or RH (current opengl default). Probably most OF users come from processing, and they'd expect processing like behavior (LH). Anyone a little bit more advanced, or comes from OpenGL, I'd expect should probably already be aware of handedness and how to fix issues when switching.

When I wrote ofCamera many years ago I brought this up, because we were also writing ofMesh at the same time, and these problems were starting to raise their head. (in fact I can see my comments regarding handedness are still in the header of ofCamera! :)

I think OF should be handedness aware. It's really not very difficult. ofCamera should know whether it's left handed or right handed and set the winding and culling flags appropriately. Also if you load an fbx OF can automatically decide to flip the winding or not, because it knows it's own handedness.

Also I think helper functions to setup different types of cameras would be very helpful, both for advanced people and beginners:
ofCamera::setup(handedness, upVector, originOnScreen, scale); // handedness is enum for LH or RH, upVector is enum of +X, +Y, +Z, -X, -Y, -Z OR a vector.

and then we could have further helper functions which call the above with correct origin, scale and upvector parameters to have even more control and mapping to common coordinate systems:

ofCamera::setupOpenGLCamera(); // RH, [0, 0, 0] in screen center, 1 unit = 1/2 screen width
ofCamera::setupProcessingCamera(); // LH, [0, 0, 0] in screen top left, 1 unit == 1 pixel
ofCamera::setupDefaultOFCamera(); // same as above
ofCamera::setupUnityCamera(); // LH, XZ Floor, Y Up, [0, 0, 0] in screen center
ofCamera::setupC4DCamera(); // LH, XY Floor, Z Up
ofCamera::setupMayaCamera();
ofCamera::setupUnrealCamera();
etc.

If we are maintaining a global space design pattern, then that means functions like:
ofSetupUnityCamera();
ofSetupOpenGLCamera();
ofSetupProcessingCamera();
ofSetupDefaultOFCamera(); // same as processing, can be aliases.

To me an API like this isn't making it more complicated to the beginner, it's educating them about these issues which they ARE ONE DAY GOING TO FACE, no matter what happens. The internet is awash with "why is my model inside out" questions. It's so frustrating that the problem (and solution) is actually so frigging easy (provided our API is consistent :)

In short:
0. OF should be consistent and bug free :)

  1. All primitives winding is unified to CW or CCW. I couldn't care less which one is default. Doesn't make much difference since the culling is decided separately (with glFrontFace).
  2. Have API to support different cameras and handedness which sets winding order and culling appropriately.
  3. Choose a camera as default which gets called automatically at initialization (I would propose stick with default OF / processing style camera. No backwards compatibility is broken. Everything works)
  4. Goto the pub and celebrate.

just my 2p :)

@vade
Copy link
Contributor

vade commented Dec 8, 2014

My only concern with this, is this means you have twice the complexity in either writing an ofAddon to ensure you support both handedness (assuming you are drawing on your own, not via ofMesh).

I suppose I never bumped into the coordinate / winding issues because I never ever had the need to switch it up, not once.

@memo
Copy link
Member

memo commented Dec 8, 2014

actually if I'm not mistaken (which I very well might be :), the whole point is you don't have to address both handedness in your addon. OF does. You just wind your triangles in the agreed norm (CW or CCW, whatever is decided, probably CCW as that's the expected OpenGL default). And then OF makes sure (via glFrontFace like @tgfrerer suggests) that your faces are culled correctly. So when we're in right handed space CCW must be kept and CW culled (opengl default). If we're in left handed space (the view - and thus your winding - gets flipped) CW must be kept and CCW culled.

So we just need to agree beforehand that all OF models must have faces which are CCW (or CW. whatever is decided). For beginners everything will 'just work'. For more advanced people we need to make sure it's clear in the documentation:

  • all OF models must have CCW to be culled correctly
  • OF will take care of culling them correctly whether the coordinate system is left handed or right handed. Advanced users can obviously override all of this behaviour.

@vade
Copy link
Contributor

vade commented Dec 8, 2014

👍

@elliotwoods
Copy link
Contributor

Also from:
https://www.opengl.org/sdk/docs/man/html/glDepthRange.xhtml

image

Which means that the depth clipping could also be set to match the camera type (e.g. for DirectX the depth clipping range is reversed).

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

No branches or pull requests

8 participants