Hi Bosheng,

Thanks for your reply.  As I mentioned to ADN support after they responded
this morning saying the behavior is by design, at the very least the
documentation should be updated to remove the statement that
CRef::operator< is suitable for STL sorted containers like std::map,
because it's not.

Also, I don't think your proposed solution is suitable for sorted STL
containers which require strict weak ordering (like std::map or std::set).

I assume you mean that I could do something like this:

struct CRef_less
{
inline bool operator() (const CRef& lhs, const CRef& rhs) const
{
 return lhs != rhs && lhs<rhs;
}
};

A strict weak ordering has the following properties:

Irreflexivity: for all x, x < x is false
Transitivity: for all x, y and z, if x < y and y < z then x < z

The above predicate CRef_less is indeed Irreflexive *but is not Transitive*!

Suppose you have 3 CRefs A, B & C with the following names: "A.Material",
"B.AnotherMaterial" and "C.Material".  A and C refer to the same material
assigned to 2 different objects.  The CRefs compare equal with ==, but
obviously A compares lesser than C lexicographically.  As a result:

CRef_less()(A, B) returns true
CRef_less()(B, C) returns true
CRef_less()(A, C) returns *false*

If you were for example to do something like this:

std::set<CRef, CRef_less> mySetOfCRefs;
mySetOfCRefs.insert(A);
mySetOfCRefs.insert(B);
mySetOfCRefs.insert(C);

you would sadly discover that the set would contain 3 elements instead of
the expected 2 - not much of a set anymore, is it?

I'll stick to using GetObjectID for sorted containers, though it can be
worth adding a check for inequality if you expect lots of duplicates since
it can early out and prevent constriction of 2 ProjectItems:

struct CRef_less
{
inline bool operator() (const CRef& lhs, const CRef& rhs) const
{
return lhs != rhs && ProjectItem(lhs).GetObjectID() <
ProjectItem(rhs).GetObjectID();
}
}

-Nicolas



On Mon, May 7, 2012 at 9:38 PM, Bosheng An <bosheng...@autodesk.com> wrote:

> Hi Nicholas,
>
> My name is Bosheng and I am a developer from Softimage.
>
> Thank you for your statement here and your mention is true, but I think we
> designed these comparison logic purposely. We just defined our CRef ">" and
> "<" that way.
>
> I guess what you would like to do is twisting the predicate in your sort
> function a bit to: using "==" to check whether the two operands are the
> same object before invoking ">" or "<".
>
> Sorry for the inconvenience.
>
> Thanks and Regards,
>
> Bosheng.
>
>
> From: softimage-boun...@listproc.autodesk.com [mailto:
> softimage-boun...@listproc.autodesk.com] On Behalf Of Nicolas Burtnyk
> Sent: Saturday, May 05, 2012 7:31 AM
> To: softimage@listproc.autodesk.com
> Subject: Re: Why is Material::operator& private?
>
> Well I'm glad this list exists!  It's great for bouncing ideas around and
> the fact that the Soft devs are actually active here is fantastic!
> I hope I can start to contribute more and be helpful to others.
>
> On Fri, May 4, 2012 at 3:59 PM, Raffaele Fragapane <
> raffsxsil...@googlemail.com<mailto:raffsxsil...@googlemail.com>> wrote:
>
> I don't know who you are Nicolas, but I'm suddenly glad we have you around
> on the list and using Soft :)
> On May 5, 2012 2:21 AM, "Nicolas Burtnyk" <nico...@redshift3d.com<mailto:
> nico...@redshift3d.com>> wrote:
> Unfortunately, that doesn't work.  CRef::operator< performs a
> lexicographical comparison using CRef::GetAsText().
> In my scene when I have the same material assigned to multiple objects,
> the CRefs I get back from X3DObject::GetMaterials() give me different
> results from GetAsText even though they compare equal (and they are indeed
> the same material).
>
> Example:
> A bunch of objects with "Scene_Material" assigned, and 1 object (torus)
> with "Material" assigned.
> This is the result of calling GetAsText on all the materials I get from
> calling GetMaterials on all the meshes in the scene.
>
> //all materials (unsorted)
> Model1.sphere.Scene_Material
> Model.cube.Scene_Material
> Model.cylinder.Scene_Material
> Model_Instance.torus.Material
> cone.Scene_Material Material
> disc.Scene_Material Material
>
> These materials are in a std::vector<CRef> called "materials".
>
> I then want to remove duplicates, so I call:
>
> std::sort( materials.begin(), materials.end(), CRef_Pred::less() );
> materials.erase( std::unique( materials.begin(), materials.end(),
> CRef_Pred::equal() ), materials.end() );
>
> where the predicates here evaluate to CRef::operator< and
> CRef::operator==, respectively.
>
> In case you're not familiar with the stl algorithms, std::unique removes
> duplicates from a sorted range by moving them to the end of the container
> and returning the new end of the range with no duplicates.  The important
> thing is that the range must be sorted for the algorithm to work correctly.
>
> Turns out that the lexicographical comparison is not appropriate for CRef
> sorting since 2 CRefs can be both equal and less than at the same time!
>
> CRef m1, m2;
> m1.Set(L"cone.Scene_Material Material")
> m2.Set(L"disc.Scene_Material Material")
> m1 == m2; //true!
> m1 < m2; //true!
> assert( (m1 == m2) != (m1 < m2) ); // <--- Wrong assumption!
>
> So, as a result, calling std::sort on my vector of CRefs doesn't correctly
> sort the vector:
>
> //all materials (sorted)
> Model.cube.Scene_Material
> Model.cylinder.Scene_Material
> Model1.sphere.Scene_Material
> Model_Instance.torus.Material  <-- Oops!
> cone.Scene_Material
> disc.Scene_Material
>
> And therefore my removal of duplicates doesn't work correctly:
>
> //all materials (unique)
> Model.cube.Scene_Material      <-- !
> Model_Instance.torus.Material
> cone.Scene_Material            <-- !
>
> So CRef::operator< is not suitable for sorting.  On a side note, that
> means it's also not really suitable for use in a map (the line in the docs
> "This is useful for sorting operations with stl maps." is not correct.).
>
> -Nicolas
>
>
> On Fri, May 4, 2012 at 4:27 AM, Marc-Andre Belzile <
> marc-andre.belz...@autodesk.com<mailto:marc-andre.belz...@autodesk.com>>
> wrote:
> Do you really need to instantiate ProjectItem objects ? Try using
> CRef::operator < in your predicate instead.
>
> -mab
> ________________________________
> From: softimage-boun...@listproc.autodesk.com<mailto:
> softimage-boun...@listproc.autodesk.com> [
> softimage-boun...@listproc.autodesk.com<mailto:
> softimage-boun...@listproc.autodesk.com>] on behalf of Steven Caron [
> car...@gmail.com<mailto:car...@gmail.com>]
> Sent: Thursday, May 03, 2012 8:46 PM
> To: softimage@listproc.autodesk.com<mailto:softimage@listproc.autodesk.com
> >
> Subject: Re: Why is Material::operator& private?
>
> in a rendering context speed is paramount, keep up the good fight! and the
> pressure on the autodesk to make the improvements you need.
>
> On Thu, May 3, 2012 at 5:41 PM, Nicolas Burtnyk <nico...@redshift3d.com
> <mailto:nico...@redshift3d.com><mailto:nico...@redshift3d.com<mailto:
> nico...@redshift3d.com>>> wrote:
>
> the on-the-fly construction of 2 ProjectItems to do the comparison is a
> lot slower than if I had those ProjectItems (or Materials or whatever)
> already.  Slow is a relative term so let me be more specific - on my
> machine, constructing 100 Materials from 100 CRefs and calling GetObjectID
> takes about 0.16ms whereas simply calling GetObjectID on 100 Materials take
> only about 0.001ms.  Probably not a big deal :)
>
>
>
>

Reply via email to