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

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