Disposition: ?
OMG Issue No: ?
Title: CadConnection, CadSystem: Avaliable_models() enhancement
Source:
Markus Helmke, Produktionstechnisches Zentrum (PTZ)
Technische Universität Berlin
Email: markus.helmke@ipk.fhg.de

Summary:

The method "available_models()" provides access to models available to the active CAD System and returns all available models in a list. Up to now the method does not take into account how the models are structured on the server side. When a user selects a model it would be helpful to navigate through the tree structure of the servers file system. A similar problem is the specification of a users file name in that moment when he wants to use "save_model_as()" or "create_model()".
The proposed methods, data structures and definitions aim to improve navigation capabilities for both systems and users when accessing models. For various reasons a native CAD-system normally is able to access different directories which are presented also to the user. This is to let him structure his work but also in order to offer templates that ease the design process, because in most cases designer aren't starting from scratch, but they are provided with templates that cover predefined parameters or a special design philosophy.
Assuming a user wants to store, create, or open a model on the (maybe remote) machine running the Cad-Services he expects something like a "FileChooser"-dialog. The method "available_models()" was not designed to support such functionality and therefore needs to be extended.
The proposed methods allow for implementing the intended functionality because they represent an essential subset of methods needed to do the following:

• Navigation (get_root_directories(), get_models_and_folders(), get_parent_directory())
• Create new subfolder (create_new_folder())
• Providing information to choose a file or folder on the client side (get_models_and_folders())
  The concept provides the possibility not to expose the whole file system to the user by using "root_directories". Every directory can serve as root directory and thus only its subdirectories are exposed to the user. The user starts navigation in the one of the roots.
  The methods were taken from JAVA's abstract class "javax.swing.filechooser.FileSystemView" which serves as "JFileChooser's gateway to the file system" and as such promise to be sufficient. Additionally IWF has implemented against the proposed CAD-Services enhancement with success.

Resolution:
?

Revised Text:
The following typedefs, struct and operations are added to the CadConnection module respectively to the CadSystem interface:

module CadConnection
{

struct Path

;

typedef sequence<string> RootsList;
typedef sequence<Path> PathList;

interface CadSystem

;
};

Disposition: ?
Disposition Parameter: ?

see below

Suggest altering the input parameter CadFeature::ParameterSeq model_params on CadConnection::CadSystem to a CosPropertyService::Properties

This issue was previously (v 1.1) resolved

I suggest altering the use of EntitySeqs to LongSeqs - essentially to use Unique IDs as identifiers. [In places like: Model::delete_entity(), EntityGroup::add_entities(), EntityGroup::remove_entities()] I see there 2 main problems: 1. I don't see that it is really neccesssary to use the (in this case Entity objects) CORBA objects to identify an object. unique_id's could always be used. -> Less traffic and a lot easier to map. Besides: how much sense does the UidUnsupported exception make? unique ids can always be created. 2. The required mapping especially on the server side is somewhat tricky and not the fastest. You would have to map the CORBA objects to the implementation objects. Nasty thing to do.

see below

'CadMain::ModelInstance::component()' collides with the new 'component' keyword for IDL from CCM.

I suggest ro rename it to 'CadMain::ModelInstance::component_model()'.

see below

is there any reason, why the double parameter (tolerance) of CadGeometry::Curve::tessellate() is declared as an in parameter and not inout? In other places like CadGeometry::Surface::nurbs_representation() the tolerance parameter is declared inout.

see below

The distinction between model instances and top-level entities was unclear to me, especially so given my task of trying to write a general purpose tool to "import" arbitrary CAD models into CFD-GEOM via the CAD Services interface. A Model can contain zero or more ModelInstances, and this provides support for assemblies. At the same time, certain entities in a Model are designated as top-level entities, which also brings to mind the concept of a composition or assembly.

Additional text is needed in the documentation

It's necessary to add some operations to CadFoundation module's interfaces:

CadFoundation::Entity::SetColor(CadUtility::ColorStruct color) raises (CadUtility::CadError);
CadFoundation::Layer::SetColor(CadUtility::ColorStruct color) raises (CadUtility::CadError);

The recommended changes are made as suggested in the summary

The Entity::euclidean_dimension() operation is described as returning the "Euclidean dimension of the entity" (with type long). What is this referring to? My guess is that it returns 2 for two-dimensional entities or 3 for three-dimensional entities, but this is unclear

Additional documentation is needed

I was already familiar with the Factory design pattern and so the purpose of the EntityFactory and ModelInstanceFactory interfaces was immediately obvious to me. I'm not sure how familiar other users of the CAD Services will be with this pattern, depending on their software development background.

The documentation shall clarify the intent of these "Factory" interfaces

There are some different tessellation structures for different kind of entities (geometric and topologic) and some are for both (e.g. EdgeTessellationStruct). Three of the four structs have a obj_ref member with type Object.

Issues: 1. The type Object for obj_ref is too generic. 2. Some of the tessellation structs are for the use with one single interface (ConnectedFaceTessellationStruct -> Body) while some others are used in more than one interface (EdgeTessellationStruct -> Edge, Curve). 3. In some cases the obj_ref member of a single tessellation struct references a geometric entity (Curve, Surface), in others a topologic entity (Edge, Face, Body)

Proposed Solution: For each interface which provides tessellation (has a tessellate() method) there should be one corresponding tessellation struct with a obj_ref member of the type of the interface which generated the struct.

Edge -> EdgeTessellationStruct Face -> FaceTessellationStruct Curve -> CurveTessellationStruct Surface -> SurfaceTessellationStruct Body -> ConnectedFaceTessellationStruct

It would be much clearer that way and the redundancies pretty few.

In CadFoundation::EntityPropsStruct one variable is named ref_position in the pdf and ref_positions in the idl files. Should be ref_position

The CadBrep::OrientedEdge interface has two very similar methods to get the Face/OrientedFace which uses the OrientedEdge. That's pretty redundant. Besides: Both methods deliver useful information only when the OrientedEdge belongs to one and only one Face. Otherwise an exception is generated. I don't really see the usefulness of those two method, especially since for many cases there are two Faces for the Edge and one can always traverse there via the EdgeLoop.

In the CadBrep module there are almost always two ways to traverse the topology: from the top level entities like Body to the underlying entites like Shell, Face, Edge, ... and the other way. For example I can get from Face to the OrientedFace which aggregates the Face or I can ask the OrientedShell in which Body it is contained. This does not make much sense to me. I always thought the topology is more or less hierarchical with Body is composed of OrientedShells which has reference to Shell and so on.
Unless I am missing something important, it is just some not so small overhead to have all those relations at hand and being uptodate with them. I would place such things in a client for I see it as application specific, if it is neccessary or not.

2. The Documentation for CadMain::Model::unique_entities() and CadMain::Model::top_level_entities() says the only valid types asked for are geometric entity types but there is only mention and used CadFoundation::Entity (in the doc and in the return types). If this should really be only geometric entities, the return types could be CadBrep::BrepEntity and the documentation should be much clearer on what is allowd and what a invalid type.

CADServices 1.1
1. In The CadMain::Model interface there is no clear way to get the DesignFeatures of a Model. The only possible way would be via top_level_entities() or unique_entities(), but these calls are for geometric entities (so it is written in the documentation).
Proposed Solution: add a call "CadFeature::DesignFeatureSeq design_features()" to CadMain::Model. The DesignFeatures should be more clearly seperated from the BrepEntities as they are to different sights on the same data and should not be mixed per chance.

I have a problem understanding how to interpret the
> vertex_number's for
> the points in CadGeometry::EdgeTessellationStuct. I don't see
> the need
> for a indexing here. It is just sequencial. Are there people,
> who want
> to do this another way?

In the PDF document (03-02-02) for the CadServices the return value for CadFoundation::Entity::reference_position is CadUtility::PointStruct, but in the idl files it is CadUtility::PointStructSeq.
I couldn't verify it against the newest documents (maybe newer than mine), because they were unreachable.
Besides, the description of this method is not very helpful (at least to me). I have no clue how to interpret it.

after changing the fourth parameter of CadConnection::CadSystem::create_model from CadFeature::ParameterSeq to CosPropertySevice::Properties recently, the exception CadConnection::BadParameter does not match the method anymore.
It should either be changed in a similar manner or kicked out completely (it is only used by create_model).

The return value of CadFoundation::Entity::cad_model() is Object. It would be clearer if it would return CadMain::Model (forward declaration).

The method CadMain::Model::unique_ids_to_entities() can throw the CadMain::NotValidCadType exception but unlike the other methods able to throw this does not get TypeCodes as input. If the exception is there by accident I suggest it be removed.

In the PDF document for CADServices 1.1 (03-02-02) the description for CadMain::Model::unique_ids_to_entities() is missing

The interface Parameter provides four functions · string get_expression() raises (CadUtility::CadError); · void set_expression(in string e_value) raises (CadUtility::CadError); // operations to allow an expression that may drive geometry

· CadUtility::EntityAttrib get_value() raises (CadUtility::CadError); · void set_value (in CadUtility::EntityAttrib value) raises (CadUtility::CadError); // operations providing access to parameter value Actually “Parameter” is an easy interface. But since we have seen different implementation approaches and since it was not clear for us in every case too what should be handed over to the set-methods and what could be retrieved by the get-methods I write down our assumptions and hope that it will be corrected if that was not the approach you have had in mind: A parameter covers expressions like “a=6” or “a=b+c”, where “a” is the “name” of the expression. getValue() therefore will return the recursive computed value in the case of “b+c” or an error when there is a problem with the computation. In case of “6” getValue() will just return the value “6”. setValue() allows to set a value if the value of “is_read_only” has the value “false”. The type of the handed over value must match the type of CadUtility::EntityAttrib stored by parameter. In the case of “a=6” “is_independent” keeps the value true. In the case “a=b+c” “is_independent” turns to the value “false”. It is a question whether handing over a value like “8” should be allowed at all when the parameter carries an non numeric expression. getExpression() is similar to getValue() with the difference that the return type is a string. setExpression() allows to set expressions like “d+e” or “8”. The values of “is_independent” and “is_read_only” have to be set accordingly. Expression like “a=d+e” and “a=8” should be possible but internally the expression is parsed and “a=” is ignored. Expression like “b=d+k” should be forbidden because they provide a back door to define completely new parameter when name is set to “b”. If this also has been intended this should be made clear. But I guess this would require extra methods that allow to define parameter. Summing up it is actually no problem to omit the value-methods The “expression”-methods can take over the task to handle values because these also can be represented by expressions like “8”.

see below

The interface "Cadsystem" provides access on a "CadUserInterface" by the
method

boolean is_interactive(out CadUserInterface gui);

Normally loading a model in the CAD engine and showing it with the editor
are two different tasks.
"create_model" and "open_model" obvioulsy are related to CAD-Engine
activity. GUI activities like showing, hiding etc. are not mandatory
implicated. If methods like "create_model" and "open_model" are intended not
to include GUI operation then functionality to control the
"CadUserInterface" is missing.
Therefore the following function would be very helpful (They assume that a
model is already loaded by the CAD engine):

createWindow(model) // Creates a window and put it in the foreground
hideWindow(model) // hides an existing window but doesn't
delete it
foregroundWindow(model) // put an existing (even if hided) window in the
foreground
deleteWindow(model) // delete the window and the corresponding window
data structure but NOT the model within the CAD engine

Every model should throw a "ModelNotLoadedException" if the model is not
loaded.

Summing up this should allow for a separated control of activities within
the CAD-engine and the editor.

see above

In some methods (mainly Model-Interface) "Entity-Types" are demanded as an
selection mechanism to select appropriate entities. Example:

CadFoundation::EntitySeq top_level_entities (
in CadUtility::TypeCodeSeq entity_types)

But the CAD-Services provide no opportunity to implement a method that
allows to ask for these entity types that are supported by the respective
CAD-System. But in our mind it is up to the CAD-System and thus to the
CAD-Services to provide such information

see above, issue rejected

The Edge::start_parameter() and Edge::end_parameter() operations return the "curve parameter corresponding to the start (end) vertex". I suppose this is referring to the parametric distance along the underlying curve, but it's unclear because of the sort-of generic use of the word "parameter".

see above

For example, consider the requirements for calling Body::tessellate():
ConnectedFaceTessellationStruct
tessellate(in TessType t_type,
inout TessParametersStruct params,
out boolean flag);

The first argument (the tessellation type) can be one of two enumerated values, WIREFRAME or VISUALIZATION. Nowhere in the specification is any hint given as to the difference between these two settings. One might guess that the VISUALIZATION tessellation type produces a larger number of triangles, but to what degree? Is it implementation-dependent?

see above

I propose to remove CadGeometryExtens interface:
module CadGeometryExtens{

interface Point : CadFoundation :: Entity

;

struct PointOnSurfaceStruct

;

interface PointOnSurface : Point

;

struct PointOnCurveStruct

;

interface PointOnCurve : Point

;

It seems to be quite unuseful

see above

interface Model : CadFoundation:: Attributable

was replaced by

interface Model : CadFoundation:: Entity

This is an issue I've posted many times ago. This really simplify the
life with implementation and classification of entities.

The inheritance for Model is changed as suggested

struct PolyLineStruct is defined but not used anywhere. Either the according interface PolyLine is missing or the struct is obsolete

PolyLineStruct is obsolete and should be removed.

CadFoundation.idl: In the CadFoundation::EntityPropsStruct the variable global_position is never mentioned in the pdf file. Besides: The corresponding method to global_position in CadFoundation::Entity is named global_location. Shouldn't that be either both position or both location?

see below

An empty TypeCodeSeq in Model::bounding_box() and Model::top_level_entities() will be handled differently according to the comments in the IDL-files. In bounding_box it means all entities and in top_level_entities it should generate a BadParam exception. Would it not be better, to give it in both methods a similar meaning?

Recommend "bounding_box" behavior.

see above

void add_child(in Model child_model) raises (CadUtility::CadError);
void remove_child(in Model child_model) raises (CadUtility::CadError);
It is suggested that Model be changed to ModelInstance to allow for assemblies

see above

The existing interfaces on the CadSystem interface do not indicate CadMain::Model references that may be opened and available for immediate use

see above

The Layer interface needs a set_color() operation

see above

adGeometry IDL comment documents the 'angle' field in CadGeometry.TessParametersStruct
as "deviation between normals of facets".

Neither the IDL nor the CAD Services spec. indicate what angular units should be used.

This oversight was corrected by indicating the required use of degrees for angle units.

The type of the 'normals' field in TessellationStruct should be VectorStructSeq
instead of PointStructSeq.

Agreed. IDL modified.

Should there be a get_parameter_set function at the Model interface level?

see above

CadBrep Edge.nearest_points function needs clarification for
'nearest_points' argument.

How are the nearest_points defined? It is declared as a PointStructSeq,
but no further details are provided. Will there always be 2? Is the
first point of the sequence a point on the given edge and the second
point a point on the edge corresponding to the input argument? What is
the expected behavior if there are multiple locations that the 2 edges
have an identical minimum distance?

see above

CadBrep Edge.nearest_points function incorrectly defines the
'nearest_points' argument as 'in'.

CAD Services spec documents Edge.nearest as 'Determines the minimum
distance, and corresponding closest points, between this Edge and the input
Edge. The returned value is the minimum distance'. The 'nearest_points'
argument should be declared as 'out'.

Resolved through removing the nearest_points operation. See issue 4493.

CadBrep Edge IDL defines a 'start_vertex()' function, but not
an 'end_vertex()' function.

Agreed. An operation was added to the Edge interface.

CadBrep Oriented Edge IDL defines an 'end_vertex()' function,
but not a 'start_vertex()' function.

Agreed. An operation was added to the OrientedEdge interface

Binding for Face.nurbs_representation needs major design overhaul or
removal.

IDL indicates the function returns a CadUtility.NurbsSurfaceStruct. A
simple NURBS surface structure cannot represent arbitrary faces w/
irregular boundaries, both outer and inner.

see above

CAD Services specification needs to clarify 'exact' for
nurbs_representation functions.

nurbs_representation indicates that if the NURBS is exact, then tolerance
is returned as negative value. Is that exact geometrically or exact
geometrically AND parametrically?

see above

Edge.nurbs_representation should h. 'tolerance' argument declared as
'inout' instead of 'in'.

CAD Services specification indicates that 'If the representation is exact -
tolerance will be returned as a negative value.

Agreed. This corrections involved a minor modification of the IDL.

Surface.nurbs_approximation should have 'tolerance' argument declared as
'inout' instead of 'in'.

CAD Services specification indicates that 'If nurbs representation is
exact, tolerance will be returned as a negative'. By the way, the 'NURBS'
acronym should be capitalized in the spec and IDL when used in this
context.

see above

Surface.nurbs_approximation should be renamed nurbs_representation to be
consistent w/ Curve.nurbs_representation.

Agree. Representation is a better description

I was building a Java3d display for CadServices data and I realized that
> we do not return the object color in the PresentationStruct. We return
> the color of the highlight (specular_color), and various parameter about
> reflectance - but we do not include the base color of the object.
>
> I think we need to add a "ColorStruct color;" to the PresentationStruct.

see above

• IDL extracts in the document are inconsistent about showing module
  declarations.

Module declarations added to the document

UML looks like Corba profile, but is not quite. In particular the
<<Interface>> stereotype should be <<CORBAInterface>>. There may also
be some "in" tags in the wrong place.

CORBA UML diagrams throughout the document were updated to an appropriate format

Mikhail Kazakov mentioned that some of the Cad Services interfaces may be under-specified. Here is a list of operations needing additional documentation:
1. CadMain:: open_model( …) - the question involves whether this operation loads the specified model into memory or are all the available models in memory and the open_model() operation merely returns an object reference to the model.

Recommendation: the documentation should include the following: The open_model() operation loads the specified model into memory and can be unloaded by invoking close_model() on the Model object reference.

2. CadMain:: top_level_entities (in CadUtility::TypeCodeSeq entity_types) - what happens when an empty TypeCodeSeq is passed? One could respond with all top level entities or none.
Recommendation: Documentation shall state that an empty TypeCodesSeq will result in a system exception (BAD_PARAM).

3. unsigned long unique_entities_count (in CadUtility::TypeCodeSeq entity_types) raises (NotValidCadType, CadUtility::CadError); - what happens when an empty TypeCodeSeq is passed? One could respond with all entites or none.
Recommendation: Documentation shall state that an empty TypeCodesSeq will result in a system exception (BAD_PARAM).

4. CadFoundation::EntitySeq unique_entities (in CadUtility::TypeCodeSeq entity_types) raises (NotValidCadType, CadUtility::CadError); – what happens when an empty TypeCodeSeq is passed? One could respond with all entites or none.
Recommendation: Documentation shall state that an empty TypeCodesSeq will result in a system exception (BAD_PARAM).

The identified recommendations were agreed.

double inertial_moment_spherical;
double gyration_radius_spherical;
CadUtility::VectorStruct first_moments_centroidal;
CadUtility::VectorStruct inertial_moments_centroidal;
CadUtility::VectorStruct inertial_products_centroidal;
CadUtility::VectorStruct principle_moments_centroidal;
CadUtility::VectorStruct gyration_radii_centroidal;

are unclear.

Jim Stevens identified the following:

The "centroidal" moments are the various mass properties with respect to the center of mass for the body. This is the more "typical" case for mass properties. They correspond to similar mass property calculations in the structure that are with respect to a specific coordinate system.
Principle moments are moments inertia along the principle axes through the center of gravity of an object.
When defining a moment of inertia around an arbitrary axis thru the center of mass you end up with product terms. There is
one axis definition for which the product terms are zero, which is called the principle axis and the moments of inertia when
using that axis definition are the principle moments.
Radius of gyration is related to the moments of inertia and the mass along each axis.
I am not sure about the meaning of first moments. I am pretty sure that inertial_moment_spherical and
gyration_radius_spherical have to do with representing the model with a single sphere that would have
the same overall inertial characteristics as the analyzed body.
Jim Stephens

Based on Jim's comments I am not certain that we need the first moments or the spherical inertial moment and gyration radius.

see above

Here is a quick itemization of necessary changes (most from Sergey and
a few additional). Following the list is a modified section of the IDL
that can be used to replace the corresponding lines in the existing
CadMain.idl file. *** NOTE: Also need to add 'typedef sequence<LongSeq> LongSeqSeq;'
to CadUtility.idl.

index_faces:
Needs 'SeqSeq' of oriented_eloops instead of 'Seq'
For consistency, arg should probably be 'oriented_edge_loops' instead of 'oriented_eloops'
Does not need eloops, edges.
index_edge_loops:
Needs 'SeqSeq' oriented_edges instead of 'Seq'
index_shells:
Needs 'SeqSeq' oriented_faces instead of 'Seq'
index_bodies:
Needs 'SeqSeq' oriented_shells instead of 'Seq'
index_vertex_loops:
Ok (just included in IDL below for easier replacement of old IDL section)
index_oriented_edges
Need 'BooleanSeq senses'
index_oriented_faces
Need 'BooleanSeq senses'
No vertices or vLoops needed
index_oriented_edge_loops
For consistency, should probably be 'index_oriented_edge_loops' not 'index_oriented_edgeloops'
Need 'Seq edge_loops' not 'Seq edges'
Need 'BooleanSeq senses'
index_oriented_shells:
Need 'Seq shells' not 'Seq ofaces'
Need 'BooleanSeq senses'
No vloops needed

CadUtility::LongSeq index_faces(in CadUtility::LongSeqSeq oriented_edge_loops,
in CadUtility::LongSeq vertex_loops, in CadUtility::LongSeq surfaces)
raises (IncorrectIndex, CadUtility::CadError);
CadUtility::LongSeq index_edge_loops(in CadUtility::LongSeqSeq oriented_edges)
raises (IncorrectIndex, CadUtility::CadError);
CadUtility::LongSeq index_shells (in CadUtility::LongSeqSeq oriented_faces)
raises (IncorrectIndex, CadUtility::CadError);
CadUtility::LongSeq index_bodies (in CadUtility::LongSeqSeq oriented_shells)
raises (IncorrectIndex, CadUtility::CadError);
CadUtility::LongSeq index_vertex_loops (in CadUtility::LongSeq vertices)
raises (IncorrectIndex, CadUtility::CadError);
CadUtility::LongSeq index_oriented_edges(in CadUtility::LongSeq edges,
in CadUtility::BooleanSeq senses)
raises (IncorrectIndex, CadUtility::CadError);
CadUtility::LongSeq index_oriented_faces(in CadUtility::LongSeq faces,
in CadUtility::BooleanSeq senses)
raises (IncorrectIndex, CadUtility::CadError);
CadUtility::LongSeq index_oriented_edge_loops(in CadUtility::LongSeq edge_loops,
in CadUtility::BooleanSeq senses)
raises (IncorrectIndex, CadUtility::CadError);
CadUtility::LongSeq index_oriented_shells(in CadUtility::LongSeq shells,
in CadUtility::BooleanSeq senses)
raises (IncorrectIndex, CadUtility::CadError);

see above

5. Function
CadUtility::LongSeq index_oriented_faces(in CadUtility::LongSeq faces,
in CadUtility::LongSeq vertices,
in CadUtility::LongSeq vloops)

Maybe instead of this, the description should be something like this:

index_oriented_faces(in CadUtility::LongSeq faces,
in CadUtility::BooleanSeq senses)?

For what reason should you put the vertices and vloops in parameters?

Please see resolution in issue 4632.

4. Function
CadUtility::LongSeq index_oriented_xxxxxxxx

For functions which create oriented entities, boolean flags "sense" should be present in parameters, i.e. like BooleanSeq senses.

Please see resolution in issue 4632.

3. Function
CadUtility::LongSeq index_shells (in CadUtility::LongSeq oriented_faces)
raises (IncorrectIndex, CadUtility::CadError);

The same situation. "In" parameter orieneted_faces should be CadUtility::LongSeqSeq, because shell is a set of faces.

Please see resolution in issue 4632.

2. Function
CadUtility::LongSeq index_edge_loops(in CadUtility::LongSeq oriented_edges)

Edge loop is described by list of edges, because of this, the "in" parameter oriented_edges should be CadUtility::LongSeqSeq.

Please see resolution in issue 4632.

Function
CadUtility::LongSeq index_faces(in CadUtility::LongSeq oriented_eloops,
in CadUtility::LongSeq eloops, in CadUtility::LongSeq edges,
in CadUtility::LongSeq vertex_loops, in CadUtility::LongSeq surfaces)

Could you describe, please, the reason of presence of "eloops" and "edges" in the parameters?

Please see resolution in issue 4632

Suggest interface for STEP open shell, ie a collection of faces that do not enclose a volume.

This functionality is available in the current CadGeometry::Shell interface.

Possible resolution:

Add methods:

interface CadSystem

interface Model

This is essentially the same issue as involved with 4571.

Replace
CadUtility::PointStruct position () raises (CadUtility::CadError);
// Returns a struct of the coordinates of a single, reference location on
// the entity that should be unique relative to neighboring entities.
method by
CadUtility::TransformationStruct location () raises (CadUtility::CadError);
// Returns a struct of the applied transformation, which represents
// the location (position, turn, scale) of the entity according the global coordinates

In that case, we will be able to refer the entity according any other entities.
This method doesn't return the "last applied transformation", but "the superposition
of all applied transformations". In that case, location of the entity in 3D space will be returned.

Duplicate

There is no way to delete any CadFoundation::Entity interface child.
We shall add the possibility to delete:

Model
ModelInstance
BrepEntity
Curve
Surface

see above

There is no "3D Location" function in Entity interface.
There is only Entity.Position(), which returns PointStructure,
but it is not enough to describe a 3D transformation.
It's necessary to return TransofmrationStruct from such a function.
It's necessary also to mention, that Entity interface has a method for
transformation.

However, if Position() has another goal, than 3D location of Entity, it
shall be described in the document.

see above

The 'intersection_parameters' argument in Surface.intersect_ray() must be type 'CadUtility::UvStructSeq' instead of 'CadUtility::DoubleSeq'.

Agreed.

In the CadServer interface we have the ability to inquire launch
properties for the CadSystem, and then pass in a set of properties
when we call connect() or connect_with_password().

Once the CadSystem is running we do not have a way to query or
change the properties.

As an example - UG has a property for the search path for files.
You can set this property when you initially start up the CadSystem.
If it needs to be changed during a session the only choice is to
shut down the CadSystem with disconnect() then call connect()
again.

Also, if you connect to an already running CadSystem you
cannot determine what properties were used during the original
connect().

I think I would propose adding something like "get_properties"
and "set_properties" for the CadSystem interface. These calls
would basically allow a query and update of the properties
provided to the CadServer via the connect() calls.

CosPropertyService::Properties get_properties();
void set_properties( in CosPropertyService::Properties props );

The proposed operations were added to the CadSystem interface.

7. Creation of ModelInstances. I haven't found any way to create ModelInstance. And it's not explained in the specification.

see above

6. #include list of CadGeometry module contain:
#include "CadFeature.idl"
that has to be replaced by #include "CadFoundation.idl".
CAD Geometry never use features explicitly.

This is a good observation. The #include was modified.

5. Method names in topological entities are not everywhere unified. To return a single entity we use normally prefix get_entity.
To return a sequence we use no prefix normally: for example

Shell get_shell () raises (CadUtility::CadError);
OrientedFaceSeq oriented_faces () raises (CadUtility::CadError);

Not all method names in CadBrep module satisfy this rule.

All operations that did not follow the standard naming procedure were changed.

3. It is not clearly described in the specification document, how to add new topological entities to existing model with sharing of existing entities.
Let's say we have 2 vertexes already created and we would like to add an edge.
While implementing EntityFactory interface, a developer have to care about this sharing. Otherwise vertexes won't be shared.
And from my point of view we have to speak about that in the document.

see above

OrientedEdgeLoopSeq edge_loop() raises (CadUtility::CadError); method of CadMain::EdgeLoop has to be renamed to
OrientedEdgeLoopSeq oriented_edge_loop() raises (CadUtility::CadError);
for unification.

The operation is renamed.

1. Perhaps we have to generalize is_manifold() methods in all BRep entities. I propose to have is_manifold() only in BrepEntity interface, but not in each interface. The only restriction is vertex, which can return anytime TRUE.

see above

Our entity groups in Cad Services don't have any form of an
identifier or name that can be retrieved through the interface.
The only returned data is either a list of entities in the group or
the count. It makes it difficult to present this information to a
user.

We might consider adding an attribute to the entity group,
however that means it gets inherited to the layer.

readonly attribute string native_label;

A readonly attribute was added to the EntityGroup interface.

Section 3.4, Editorial: Although the description of interaction with
PDM Enablers is fairly clear, a sequence diagram (or several, for
the several cases) would increase the likelihood of
interoperability.

Appropriate sequence diagrams were added to the specification.

Sections 1.8, 2.8, 3.1, 3.2, Editorial: Compliance points / levels
and optional interfaces are spread around the document in a way that
would hamper formal claims of conformance.

see above

Support for STEP input / output?

see above

Body.property_info, several values are calculated. What is the return accuracy? Should a separate accuracy for each separate property be returned? Should accuracy be returned at all?

see above

Should Shell.tessellation return a similar structure to Body.tessellation? Currently shell tessellation returns a FaceTessellationStructSeq while body tessellation returns a BodyTessellationStruct. Shell tessellation results are the only tessellation results that do not have an object reference to underlying topology. Rename BodyTessellationStruct to something like ConnectedFaceTessellationStruct?

see above

Model level parameters for geometry creation. Should the model creation operation support new parameters?

Agreed. A Model creation operation was modified in the CadSystem interface.

Where can one get the CadFeature::Parameter instances from to use in CadSystem::create_model() ?
As far as I know, the instance of a idl interface can only be created on the server (in a get method or in a factory) and I can see no way of creating empty CadFeature::Parameter instances.
Besides, why was CadFeature::ParameterSeq used on Model level (CadMain::Model::create_model(), CadMain::Model::get_parameter_set()) and not as in other places CosProperyService::Properties? It is not clear to me, what information is supposed to be behind these parameters.

This looks like a valid concern. We can fix this by using either CosProperyService::Properties in place of the CadFeature::ParameterSeq or we can created a CadFeature::ParameterFactory.

I suggest the use of Properties.

File CadMain:

1. Method add_child and remove_child from CadMain::Model should has
CadMain::ModelInstance in parameter instead of CadMain::Model.

File CadGeometryExtens:

1.
struct SurfacePatchStruct

;
Should be
struct SurfacePatchStruct

;
because parent_surface can be any type.

struct OffsetCurveStruct

;
Should contain also field CadGeometry::Curve BasisCurve.

struct HyperbolaStruct

;
Should be moved from module CadSurface to CadCurve, because it is used
only by "curve"
interfaces.