Subversion commit 5800.
Made some trivial changes to the perl & python binding code to follow the recent
API changes.
Note neither can build because they have not been updated to the new
import_finite_element (and fieldml) functions which take the region as an
argument rather than returning it.
Looks like could benefit from calling external API function
Cmiss_region_read_file - if that can be made to automatically load fieldml.
(Admission: I don't know how to build them...)

Subversion commit 5799.
Replaced Cmiss_region_begin/end_change_API with internal versions (no _API
suffix) which now also begin and end field manager caching.
Fixed exports for above two functions and Cmiss_region_create (was
create_Cmiss_region which should not be exported).

If we adopted Doxygen for our API we could use some custom tags to denote
status, e.g.:
prototype - new and subject to change... beware!
published - current API
deprecated - we will soon remove it
---
I don't want to promise binary compatibility on the current API, but it's close
to being 'acceptable'.
Once we accept a minimum set of published API (i.e. enough to perform real
tasks) then we may promise binary compatibility on published API.
We can then introduce a policy of leaving deprecated API working as long as
possible and hopefully only break BC on major version changes.
If you are using prototype API then expect to require rebuilding.
---
API changes could go in the CHANGELOG, or perhaps we should add a more specific
APICHANGES file?
It would be cleaner to start once we have accepted a minimal, published core
API.

Created by blackett on 2008-06-04 11:31
:I don't know of anyone who will be bitten if we break binary compatibility
here.
:I think the only people using the old way were PCenv which no longer uses cmgui
:and others who were instructed to use computed variables at the time, which we
:have already broken.
:I think we should just go ahead and provide a good example of how to use
:the API to make and work with some regions/fields.
and, of course, document the API changes somewhere obvious for us users to find
:)

I don't know of anyone who will be bitten if we break binary compatibility here.
I think the only people using the old way were PCenv which no longer uses cmgui
and others who were instructed to use computed variables at the time, which we
have already broken.
I think we should just go ahead and provide a good example of how to use
the API to make and work with some regions/fields.

I will update cmgui.exports to have Cmiss_region_create instead of
create_Cmiss_region.
It also looks like the real Cmiss_region_begin/end_change should now be exposed
by the API.
Do I need to maintain source compatibility with the following?
#define Cmiss_region_begin_change_API Cmiss_region_begin_change
#define Cmiss_region_end_change_API Cmiss_region_end_change

Subversion commit 5794.
D'oh! Was interpreting 0 return_code from fieldml SAX parser as failure, when it
was in fact successful.
Hopefully all examples pass tests now.

Even though we intend to migrate users to calling the API, I still want to
update 'gfx define field' to work with regions.
My simple idea is to allow the name to be preceded by the region path which then
is the source of the computed field manager for the source fields, which will
suffice in most cases:
gfx define field REGIONPATH/NAME ...
Note I'm not yet ready to allow a field to depend on source fields from other
regions... let's work through the use cases carefully (embedding for example).

Subversion commit 5793.
Calling new Cmiss_region_detach_fields_hierarchical to remove circular
references before deaccessing root_region in final clean-up of command data.
Note this is still a workaround - won't handle cases where child regions are
deaccessed by last parent while fields (in the orphaned region) still access it
or a child region.

A better workaround for the leaks (4/5 remaining failed examples) is to call
some new function:
Cmiss_region_detach_fields_hierarchical()
...which goes through and DEACCESSes region->fields throughout the hierarchy.
Then in theory I can restore DEACCESS(Cmiss_region)(command_data->root_region)
as the cleanup.
Calling DESTROY(Cmiss_region) directly caused additional problems with the
time_dependent_transformation test because Cmiss_region_get_FE_region was being
called by field clean up code after the any_object list was cleaned up.
I will try this now.
Only remaining break is then a1, reading fieldml.

I should have mentioned that I am following boost terminology here, where a
weak_ptr is one that can be converted to an accessed real pointer (boost
equivalent: shared_ptr) while the real object is alive, but the conversion/cast
returns NULL after the real object is destroyed.

Current plan for circular references: those fields that currently ACCESS a
Cmiss_region, but request callbacks from an FE_region can:
(1) Access the FE_region instead (otherwise it is not possible for them to clean
up callbacks in time, and
(2) Maintain a "weak pointer" to the Cmiss_region - see below.
---
Weak Pointer implementation idea (feel free to comment):
~~~
Consider Cmiss_region to have a [strong] access_count and a weak_access_count,
both set to 1 on create.
struct Cmiss_region_weak_ptr *Cmiss_region_weak_access(region);
... will return the Cmiss_region cast to a Cmiss_region_weak_ptr, with
weak_access_count incremented.
To use the region pointed to by the weak_ptr, call:
struct Cmiss_region *Cmiss_region_weak_ptr_access_object(cmiss_region_weak_ptr);
... which will return a struct Cmiss_region with an incremented access_count,
which you need to DEACCESS when no longer using.
... otherwise NULL if the region has been destroyed (access_count == 0).
DESTROY(Cmiss_region)
... only cleans up the contents of the region, not the main structure, and
finally calls the following:
int Cmiss_region_weak_ptr_deaccess(&cmiss_region_weak_ptr);
... which reduces weak_access_count (also called by clients holding weak_ptrs to
clean up)
... and if weak_access_count goes to zero it frees the struct Cmiss_region
(whose contents were already cleaned up)
---
Any flaws in the above?
Any alternatives?

As a temporary measure so things work at least as well as they used to (and get
the overnight tests to pass again), I will call DESTROY(Cmiss_region)() instead
of DEACCESS in DESTROY(Cmiss_command_data).
Admittedly this will not be sufficient to handle fields accessing child regions,
but these can't currently be set up.

One of the remaining issues is that some computed_fields ACCESS a region they
are working with.
This circular referencing means the final
DEACCESS(Cmiss_region)(command_data->root_region) does not clean up the region
or its subobjects.
Fields doing this are: compose, nodal_lookup, quaternion_SLERP,
quaternion_to_matrix, matrix_to_quaternion, region_sum, region_mean, region_rms.
Others access the FE_region, which isn't so serious.
Where the field is owned by the region, it needn't ACCESS it, but this may not
handle all cases.
(Graphical_elements also ACCESS the Cmiss_region; will have to deal with this
when scenes merged with regions.)

Had a look at region/cmiss_region.*.
Looks fine to me and pretty logical.
Great that we can hide the Destroy.
If I was trying to think of an interesting usecase/test, it would be to
add an example in Perl or C or whatever that used the API to
make a region, add some fields, evaluate those fields into another field or
set and get values from the nodes using the API and then write those out.
Something like this could go to resolving Andre's use case in
http://www.cmiss.org/cmgui/tracker/617 Comment #5

Subversion commit 5789.
Fixed case of full region merge into global hierarchy when no global equivalent
existed.

Transcript entry (2008-05-27 13:42) edited by rchristie

Subversion commit 5787.
- No longer an error when cannot automatically make embedded
'element_xi_coordinate' field when no coordinate field existed.
- Fixed issue where data groups were skipping the data_root_region and using
root_region as master.
Subversion commit 5788.
- Restored correct logic in write_exregion_file.
--
My apologies for committing the code with insufficient testing. Hubris took over
("what could possibly go wrong?").
I think I've fixed most of the problems, but I'm still looking into some issues
with examples a1 (reading fieldml) and e51.
I was going to back-out the changes if I hadn't got this far.

Subversion commit 5786. Fixed memory leak. My apologies.

Subversion commit 5784.
I'd appreciate review/feedback; I recommend starting with region/cmiss_region.*.
Major change is that Cmiss_region constructors add computed_field_manager and
wrap FE_region's fields behind the scenes.
This has simplified much code working with regions. File I/O now returns regions
with computed_fields; note only the FE_fields are merged at this time.
Unfortunately I still need 5 constructors to make certain Cmiss_region/FE_region
arrangements:
* FE_region merge code needs a 2-step creation - fields attached later.
* data hack region calls a special 2-step create
* groups with master region
* full region sharing basis_manager and element_shape_list with a supplied
region (needed for some I/O code)
* full region with its own bases and shapes
I will add separate tracker items on reducing this number by sorting out data,
groups and sharing of global data such as basis_manager and element_shape_list.
--
Note source/perl (Region.xs) and source/python (Region.c) code have not been
updated to use the new Cmiss_region_create() instead of several lines of code.
Anyone know how to build/run these bits?

Yep it seems better to leave data alone until we can change the fields to know
the objects they are defined on, and so we can change to the generic "points"
settings.

I'm leaving the data_root_region & synchronization with root_region as they are
now: I'm not forced to change it for this tracker item (& changes were
snowballing!).
I'm just adding another temporary function Cmiss_region_create_data_hack to
create the data_root_region.

I am thinking of making the "data_hack" FE_regions _only_ referenced from the
FE_region containing the corresponding nodes and elements, e.g. the
FE_region->data_hack_fe_region member accesses the FE_region owning the data
points.
Hence I can remove the artificial data_root_region plus descendants, and avoid
the sketchy synchronization between them and the true region hierarchy.
I wonder why I never thought of this in the first place... am I missing
something?
---
Note this follows a change in my thinking to the other alternative in the
computer science mantra "there are only two magic numbers in software: 1 and
infinity" (meaning if you have to put in infrastructure to support 2 items of a
type, you should probably allow an unbounded number of them).
Originally we were thinking there was only one list of discrete objects in a
region: nodes. Elements were treated as a different type of thing. Data was just
nodes in another region.
Recently I have begun to consider elements as being more similar to nodes:
there's no real difference between a node with time interpolation and an element
with xi interpolation.
Furthermore, in mesh-less methods we have discrete "particles" (which it seems
can have nodes and elements as well as the particles).
Hence I am now thinking you can have as many discrete objects attached to a
region as you need.
Eventually I think "node" will be just the name of a special field type which
owns discrete objects, and other specialized fields will be able to attach
values to them. The values of this field are references to the discrete objects.
"data", "element", "particle" are just additional discrete object fields.
(Currently, these discrete objects are buried in the FE_region objects attached
to the Cmiss_region.)
Comments welcome.

At this point, I'm not intending to pass around shared global FE_element_shape
list and FE_basis manager.
Each true region will have its own shapes and bases and each group using the
master's FE_region will share those with its master.
Ultimately I think these will become fields owned by some region we "import" or
otherwise reference (inherit?).
---
I'm anticipating this causing some problems with node/element import since these
objects won't match by pointer.
I suppose they could become a global singleton, otherwise we just have to match
by equivalence.

Transcript entry (2008-05-15 13:50) edited by rchristie

I intend to make CREATE(Cmiss_region) a private function, and also make it
create the region with an access_count of 1 (a la fields in tracker/657).
There will be two new public create functions:
1. Cmiss_region_create()
  This will ensure a new region is created with its own field manager and
FE_region. The field manager will automatically wrap fields from the FE_region.
  API Note: This will become the new API region create function.
2. Cmiss_region_create_from_master(master_region)
  this creates a new region referencing the same field manager as the master
plus a special group FE_region that has a subset of the nodes & elements from
the master FE_region.
  API Note: I'm not ready to have the second constructor exposed in the API as I
am looking at changing the way groups are handled (see tracker/667).
The following new or existing region functions will automatically call related
functions on the field manager and FE_region:
  Cmiss_region_begin_change(region);
  Cmiss_region_end_change(region);
  Cmiss_region_can_merge(destination_region, source_region);
  Cmiss_region_merge(destination_region, source_region);
--
After these changes are used throughout the code, file import & other region
creating operations should correctly set up regions with their own fields.

A few migration issues to work through:
- graphical element object and scene editor dialog need to hook into the correct
field manager.
- changes to commands & API.
- supporting "xi" & "cmiss_number" default fields.