Gprim & first curves

[Carsten Haubold]

Hey Bart,

I have to admit I never really used the gprim factory up to now, and on first usage I just noticed a problem:
If you merge 2 meshs containing nurbs curves, then you get 2 entries in first_curves, lets say 0 and 3.
Now your gprim_factory::add_nurbs_curve() does add its values at the end, but increases curve_count[0].
So the first group now contains 4 instead of 3 curves, but the 2nd entry in first_curves still points to 3 which means one curve will vanish.

I changed this to curve_count[first_curves.size() - 1], so the curves stay in their groups. Just for that you know :)

I noticed that as I wanted to retrieve the index to the curve being created.
DegreeElevate works for some curves, I'm not yet sure whats the problem when using circles but I'm working on it.

Cheers
Carsten

-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
K3d-development mailing list
K3d-development@...
https://lists.sourceforge.net/lists/listinfo/k3d-development

[Timothy M. Shead]

Carsten Haubold wrote:

> I have to admit I never really used the gprim factory up to now, and
> on first usage I just noticed a problem:
> If you merge 2 meshs containing nurbs curves, then you get 2 entries
> in first_curves, lets say 0 and 3.
> Now your gprim_factory::add_nurbs_curve() does add its values at the
> end, but increases curve_count[0].
> So the first group now contains 4 instead of 3 curves, but the 2nd
> entry in first_curves still points to 3 which means one curve will vanish.
>
> I changed this to curve_count[first_curves.size() - 1], so the curves
> stay in their groups. Just for that you know :)

Carsten:

Sorry about this, but through no fault of your own you just stepped on a
landmine ...

Your change is reasonable, but will likely break existing OpenCascade
and SVG code.  The issue here is that gprim_factory now gets used in two
completely-incompatible "modes" - creating new meshes from scratch (what
I intended) and adding geometry to an existing mesh (not what I
intended).  This is precisely why I didn't want to put gprim_factory in
the SDK to begin with - I anticipated that it would metastasize in
incompatible ways, and unfortunately I'm being proven right.  It's not
that there's any problem with your intended fix, it's the entire concept
of gprim_factory is fatally flawed.  It is supposed to be "a simplified
interface for adding geometric primitives to a mesh", but I've come to
realize that that is over broad in scope, and there are too many
opportunities for subtle errors and differences in expectations on the
part of callers.

There's nothing wrong with providing higher-level interfaces, but they
need to be tightly focused and well-defined.  "gprim_factory" is too
vague and could mean anything, which is why it can only lead to more
problems - it invites developers to read into it what they want, not
what it is.  So, I'm going to be more stubborn than usual and hand-out
some action items:

* Carsten: don't use gprim_factory.
* Everyone Else: decide whether you want your own copy of gprim_factory,
or to rewrite your code without it.  Report your wishes to Tim.
* Tim: remove gprim_factory from the SDK, make copies as-needed.
* Tim: provide an implementation of the Euler operators as a canonical
example of a "good" high-level API.
* Entire Team: look for situations where a narrowly-targeted API can
eliminate repetitive code / improve code clarity.  Discuss designs.

Cheers,
Tim

--
Timothy M. Shead, K-3D founder
http://www.k-3d.org



-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
K3d-development mailing list
K3d-development@...
https://lists.sourceforge.net/lists/listinfo/k3d-development

[Carsten Haubold]

Hey Tim,

sorry about that. I noticed that for better use from other plugins I'd need to have the curve remain at its current index so I dropped using gprim anyways. I'll just revert my changes and anything is fine again? ;)

Cheers
Carsten

2008/7/12 Timothy M. Shead <tshead@...>:

Carsten Haubold wrote:
> I have to admit I never really used the gprim factory up to now, and
> on first usage I just noticed a problem:
> If you merge 2 meshs containing nurbs curves, then you get 2 entries
> in first_curves, lets say 0 and 3.
> Now your gprim_factory::add_nurbs_curve() does add its values at the
> end, but increases curve_count[0].
> So the first group now contains 4 instead of 3 curves, but the 2nd
> entry in first_curves still points to 3 which means one curve will vanish.
>
> I changed this to curve_count[first_curves.size() - 1], so the curves
> stay in their groups. Just for that you know :)
Carsten:

Sorry about this, but through no fault of your own you just stepped on a
landmine ...

Your change is reasonable, but will likely break existing OpenCascade
and SVG code.  The issue here is that gprim_factory now gets used in two
completely-incompatible "modes" - creating new meshes from scratch (what
I intended) and adding geometry to an existing mesh (not what I
intended).  This is precisely why I didn't want to put gprim_factory in
the SDK to begin with - I anticipated that it would metastasize in
incompatible ways, and unfortunately I'm being proven right.  It's not
that there's any problem with your intended fix, it's the entire concept
of gprim_factory is fatally flawed.  It is supposed to be "a simplified
interface for adding geometric primitives to a mesh", but I've come to
realize that that is over broad in scope, and there are too many
opportunities for subtle errors and differences in expectations on the
part of callers.

There's nothing wrong with providing higher-level interfaces, but they
need to be tightly focused and well-defined.  "gprim_factory" is too
vague and could mean anything, which is why it can only lead to more
problems - it invites developers to read into it what they want, not
what it is.  So, I'm going to be more stubborn than usual and hand-out
some action items:

* Carsten: don't use gprim_factory.
* Everyone Else: decide whether you want your own copy of gprim_factory,
or to rewrite your code without it.  Report your wishes to Tim.
* Tim: remove gprim_factory from the SDK, make copies as-needed.
* Tim: provide an implementation of the Euler operators as a canonical
example of a "good" high-level API.
* Entire Team: look for situations where a narrowly-targeted API can
eliminate repetitive code / improve code clarity.  Discuss designs.

Cheers,
Tim

--
Timothy M. Shead, K-3D founder
http://www.k-3d.org



-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
K3d-development mailing list
K3d-development@...
https://lists.sourceforge.net/lists/listinfo/k3d-development

-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
K3d-development mailing list
K3d-development@...
https://lists.sourceforge.net/lists/listinfo/k3d-development

[Timothy M. Shead]

Carsten Haubold wrote:
> Hey Tim,
>
> sorry about that. I noticed that for better use from other plugins I'd
> need to have the curve remain at its current index so I dropped using
> gprim anyways. I'll just revert my changes and anything is fine again? ;)
Yep - as I said, this really isn't a mistake on your part, it's a
long-standing mistake on mine ...

Cheers,
Tim



--
Timothy M. Shead, K-3D founder
http://www.k-3d.org



-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
K3d-development mailing list
K3d-development@...
https://lists.sourceforge.net/lists/listinfo/k3d-development