|
Replies:
10
-
Last Post:
Sep 20, 2007 9:35 PM
by: jslott
|
|
|
|
|
|
|
First WFS implementation
Posted:
Sep 17, 2007 8:51 PM
|
|
|
Jordan has submitted his first-pass implementation of the WFS, which I have integrated into a Wonderland branch called "wfs". First of all, many thanks to Jordan for this significant piece of work. I'm personally looking forward to getting this fully integrated, because it will make building new worlds considerably easier.
To use WFS, check out the wfs branch from cvs, and edit WonderlandServerConfig.xml to specify the location of a world directory. There are a couple of sample worlds in lg3d-wonderland/src/worlds. Use "ant run-manager" to run the manager UI, which includes a button to let you reload the geometry from a WFS directory.
In the interest of getting this integrated quickly, I would like to use this thread to hold an informal code review. If you are responsible for a file that has changed, please leave any comments or questions in this thread.
The existing files that changed for WFS are:
lg3d-wonderland/build.xml
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/config/server/WonderlandServerConfig.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/client/cell/AnimatedCell.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/client/cell/Cell.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/client/cell/SimpleTerrainCell.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/common/messages/CellHierarchyMessage.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/common/messages/ServerManagerMessage.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/server/ServerManagerGLO.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/server/cell/AnimatedCellGLO.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/server/cell/AudioCellGLO.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/server/cell/CellGLO.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/server/cell/ModelViewerCellGLO.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/server/cell/SimpleTerrainCellGLO.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/server/cell/SlideShowCellGLO.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/server/cell/StationaryCellGLO.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/server/cell/UserCellCacheGLO.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/server/cell/WorldRootCellGLO.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/management/ManagerUI.form
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/management/ManagerUI.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/scenemanager/UserCellCache.java
lg3d-wonderland/src/config/WonderlandServerConfig.xml
The newly added files are:
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/common/messages/CellReconfigureMessage.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/common/messages/DataBoolean.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/darkstar/server/cell/WFSCellGLO.java
lg3d-wonderland/src/classes/org/jdesktop/lg3d/wonderland/wfs/*
lg3d-wonderland/src/worlds/*
|
|
|
|
|
|
|
Re: First WFS implementation
Posted:
Sep 17, 2007 9:18 PM
in response to: kaplanj
|
|
|
My initial comments and thoughts:
- (nitpick) In many places there are tabs instead of spaces. I don't know about other IDEs, but NetBeans has issues with the tabs.
- (bug) The modifyOnClients property of CellGLO does not work properly. The first UserCellCacheGLO that revalidates will properly send a message to the associated user, but it then sets the property to false, so no one else will get notified. The modified property must be tracked in each user's cell cache instead.
- (suggestion) The location of the WFS filesystem root could be read from a system property rather than specified in WonderlandServerConfig.xml. That way, the root could be changed in my.build.properties rather than the XML file. I think this makes sense because the server root should be a per-Wonderland-install, not a per-user attribute.
- (suggestion) I like the idea of the Properties bean class, but I don't like adding another class per cell, especially in the wfs package. Maybe properties could be "institutionalized" into the architecture? Basically, I propose we make Properties the default way to store and transmit the cell's initial state. If we do this, I believe we can create a standard form for cell setup messages, so we are back to the same number of classes per cell.
- (suggestion) I don't like the full 4d matrix in the XML file. Could we simplify it? I think we can break the matrix up into separate translation, rotation and scale pieces, which would be easier to understand and modify. I believe translation overlaps with the origin specification that is already there, and rotation and scale can both be optional. That way we can place things with just the origin, and add other parts as needed.
|
|
|
|
|
|
|
|
|
Re: First WFS implementation
Posted:
Sep 17, 2007 10:09 PM
in response to: kaplanj
|
|
|
>1. (nitpick) In many places there are tabs instead of spaces. I don't know about other IDEs, but
>NetBeans has issues with the tabs.
Yes, I was going to bring this up. Basically I just need to know the standard indent size (4?)
and whether to retain tabs or not?
>2. (bug) The modifyOnClients property of CellGLO does not work properly. The first
>UserCellCacheGLO that revalidates will properly send a message to the associated user, but it
>then sets the property to false, so no one else will get notified. The modified property must be
>tracked in each user's cell cache instead.
Very likely true, based upon my incorrect knowledge of Wonderland. I will take a look at this
issue. I think I saw the "setDeleteOnClients" flag and figured it would work similarly.
>3. (suggestion) The location of the WFS filesystem root could be read from a system property
>rather than specified in WonderlandServerConfig.xml. That way, the root could be changed in
>my.build.properties rather than the XML file. I think this makes sense because the server root
>should be a per-Wonderland-install, not a per-user attribute.
Does my.build.properties get read at run-time or just compile time? (I need to look at it further). I
think the root is a per-Wonderland-install, but something that can be changed without a
recompile. I don't know much about my.build.properties, maybe I'm being thrown off by the
"build" part of the name.
>4. (suggestion) I like the idea of the Properties bean class, but I don't like adding another class
>per cell, especially in the wfs package. Maybe properties could be "institutionalized" into the
>architecture? Basically, I propose we make Properties the default way to store and transmit the
>cell's initial state. If we do this, I believe we can create a standard form for cell setup
>messages, so we are back to the same number of classes per cell.
I agree this is an issue and don't know of a good solution (yet). I kept the Properties bean
classes separate (and WFS separate in fact) so that you can put it all in a JAR and import it
into a tool to create WFSes.
At first I thought that the Properties classes could just become "institutionalized" as you say --
but I'm not certain anymore. See the discussion below.
>5. (suggestion) I don't like the full 4d matrix in the XML file. Could we simplify it? I think we can
>break the matrix up into separate translation, rotation and scale pieces, which would be easier
>to understand and modify. I believe translation overlaps with the origin specification that is
>already there, and rotation and scale can both be optional. That way we can place things with
>just the origin, and add other parts as needed.
I agree that the XML files are too complicated. It arose from the initial design decision to make
the XML files and associated Java bean classes as similar as possible to the member
variables currently stored in the cell GLO classes. It resulted in a few things, the ugliness of
the full 4d matrix for one. Another example is the Vector3D class, which does not have the
Java Bean pattern, so the XML files must invoke its set() method explicitly.
I'm now thinking that perhaps the XML files should be designed to be 'user friendly' and make
specifying origins and bounds and the like much easier. The result of this approach is that we
would need a 'translation' step to go from the Java Bean class (as reflected by the XML files)
to the member variables stored by Cell GLO classes--we would have to translate from the
'user friendly' representation into Matrix4D and Vector3D classes at some point.
Where the Java bean classes reside is another sticky design decision -- are they part of the
Cell GLO's or are they part of WFS? It seems the more 'user friendly' we want to make the
WFS XML files, the more dis-similar they must become from the Cell GLO classes and the
more 'glue' code is needed to maintain both.
Another approach is the Java Beans become just 'collections of properties' and each Cell GLO
takes a 'collection of properties' and parses out the needed information. (Kind of like how
java.util.Properties is suppose to work I think). Then, WFS would not have to change at all for
new Cell types, and the new Cell type would just have to know how to parse out the properties
it wants.... or something along these lines.
Jonathan, many thanks for doing the branch work! I'm here to fix issues and follow-up.
Jordan
|
|
|
|
|
|
|
|
|
Re: First WFS implementation
Posted:
Sep 19, 2007 7:52 PM
in response to: jslott
|
|
|
> Yes, I was going to bring this up. Basically I just need to know the standard indent size (4?)
> and whether to retain tabs or not?
4 spaces, no tabs.
> Does my.build.properties get read at run-time or just compile time? (I need to look at it further). I
> think the root is a per-Wonderland-install, but something that can be changed without a
> recompile. I don't know much about my.build.properties, maybe I'm being thrown off by
> the "build" part of the name.
my.build.properties is a bit of a misnomer. It actually contains a combination of compile-time and run-time attributes. There is a file in the binary builds called my.run.properties that contains only the runtime properties.
> I'm now thinking that perhaps the XML files should be designed to be 'user friendly' and make
> specifying origins and bounds and the like much easier. The result of this approach is that we
> would need a 'translation' step to go from the Java Bean class (as reflected by the XML files)
> to the member variables stored by Cell GLO classes--we would have to translate from the
> 'user friendly' representation into Matrix4D and Vector3D classes at some point.
>
> Where the Java bean classes reside is another sticky design decision -- are they part of the
> Cell GLO's or are they part of WFS? It seems the more 'user friendly' we want to make the
> WFS XML files, the more dis-similar they must become from the Cell GLO classes and the
> more 'glue' code is needed to maintain both.
>
In the absence of a good solution (I don't have any great ideas), I guess I am going to vote for consistency.
Right now, each Cell has an associated CellSetup class, which is used in setup() and reconfigure(). The Cell doesn't store the setup directly, but translates it into whatever representation makes the most sense. For CellGLOs, I would propose something analogous. We can move CellProperties into the darkstar.server package, and rename it CellGLOSetup. We can also define an interface with the following methods:
- setup(CellGLOSetup) - translate the CellGLOSetup into the CellGLO's internal state
- reconfigure(CellGLOSetup) - update the internal state to match the setup
- getCellGLOSetup() - create a new setup object that represents the current state
For a cell to be supported by WFS, the developer will need to implement this interface and a corresponding CellGLOSetup type. The translation from user-friendly properties into CellGLO state can happen in either the bean, the CellGLO, or both.
I haven't come up with a name for this interface: I think the ability to read and write CellGLO state to a Java Bean is useful to systems other than WFS, so I don't think it's "WFSCell" is right. It's more like XMLSerializable, but that is realy awkward 
|
|
|
|
|
|
|
|
|
Re: First WFS implementation
Posted:
Sep 20, 2007 12:05 PM
in response to: kaplanj
|
|
|
I agree (seem to be doing a lot of that :-)) with the XML simplification. As you say it's the interface to the developer so should be as clean as possible.
One simplification we could make (from the user perspective) is to compute the cell bounds ourselves. This will require the server to load any geometry files that change and determine the bounds, removing the burden from the user (and more importantly avoiding any user error which could cause some very interesting results). We can use Java 3D with the null pipeline on the server to avoid having to install opengl on the servers.
|
|
|
|
|
|
|
|
|
Re: First WFS implementation
Posted:
Sep 20, 2007 1:27 PM
in response to: paulby
|
|
|
As a novice Wonderlander, I was often confused by the bounds property (uh, what do I set? well, I'll just set it to anything any see what happens...), so automatic computation is something I'd like to see, assuming it is not computational burdensome. Then again, it requires the server to load the geometry, can we assume it will always be available? Can't think of a reason why it shoudn't be.
Jordan
|
|
|
|
|
|
|
|
|
Re: First WFS implementation
Posted:
Sep 20, 2007 9:35 PM
in response to: paulby
|
|
|
I just realized there is another bug in the first WFS implementation -- I handle the bounds completely wrong. Like cell's origin, the origin of the bounds is given in coordinates relative to its parent. For a cell's origin, I just simply place the Cell at the leaf branch group tree hierarchy rather than always at the root in the branch group hierarchy, so that Java 3D takes care of placing the cell at the proper location in the world by applying the origin translations down the branch group chain. I need to do something so that the bounds are properly in 'world' coordinates so that the intersection operation works properly.
Seems like I can either:
a. Just convert and store the bounds of the cell always in world coordinates. This would need to get updated manually if the origin of any of the parent cells up the chain changes.
b. Modify the getVisibleCells() routine (on both client and server) to modify the bounds on the fly as we walk down the child cell chain recursively.
Jordan
|
|
|
|
|
|
|
|
|
Re: First WFS implementation
Posted:
Sep 18, 2007 7:31 AM
in response to: kaplanj
|
|
|
>(bug) The modifyOnClients property of CellGLO does not work properly. The first
>UserCellCacheGLO that revalidates will properly send a message to the associated user, but it
>then sets the property to false, so no one else will get notified. The modified property must be
>tracked in each user's cell cache instead.
I took a look and believe I just need to maintain the 'modify' bit for each UserCellCacheGLO rather than on the Cell GLO class itself. I will make the fix.
Jordan
|
|
|
|
|
|
|
|
|
Re: First WFS implementation
Posted:
Sep 19, 2007 11:21 AM
in response to: jslott
|
|
|
Firstly I wanted to say thank you Jordan for this major contribution to Wonderland. I had a chance this morning to review the code and do a little thinking....
Everything looks great. I agree with the discussions above, especially wrt to the Cell Configuration Properties. I don't have any new ideas yet to add to this except to say I don't like the idea of using java.util.Properties, I'd prefer to have the data more structured.
I think we can resolve these issues as we move forward so I suggest that as soon as WFS has the same functionality as HEAD (ie we port all of the default world into WFS) that we merge WFS into HEAD.
Rgds
Paul
|
|
|
|
|
|
|
|
|
Re: First WFS implementation
Posted:
Sep 19, 2007 7:31 PM
in response to: kaplanj
|
|
|
I've been doing some thinking about Jon's comment about simplifying the XML files and I've concluding this is fairly important to do. This will be the interface to the developer, and I don't believe we want to get a bad wrap for creating a complex, hard-to-use interface. Plus I don't want to rely upon the possible future existence of tools to make writing XML files easier.
Mulling over my mind I would like to suggest: we design the XML file format to be as simple as possible, rather than be a reflection of some member variables currently used by the Cell GLOs. As Jon suggested, that means for example breaking the Matrix4D into separate parts -- and I would take it a step further by saying we should use basic data types (int, double, string, array) wherever possible. So, an 'origin' would be given by a double array of length 3, rather than a serialized version of the Vector3D class. We would still use the XML persistence mechanism however. For example, the XML representation of a Simple Terrain cell might look like:
<?xml version="1.0" encoding="UTF-8"?>
<java version="1.6.0" class="java.beans.XMLDecoder">
<object class="org.jdesktop.lg3d.wonderland.server.cell.properties.SimpleTerrain">
<void property="bounds">
<void property="type"><string>Sphere</string></void>
<void property="radius"><double>3.0</double></void>
</void>
<void property="translation">
<array class="double">
<double>5.0</double><double>-4.0</double><double>-3.0</double>
</array>
</void>
<void property="rotation">
<array class="double">
<double>5.0</double><double>-4.0</double><double>-3.0</double>
</array>
</void>
... other transformations ...
<void property="models">
<array class="string">
<string>models/mpk20-web.j3s.gz</string>
</array>
</void>
</object>
</java>
This XML file would be backed by a "properties" class that would live in the Cell GLO package structure somewhere (hence it would be institutionalized). There is a necessary translation step between this simplified representation of the information and the Matrix4d, Vector3d, etc classes that are necessary in the 3D code. I don't believe this translation step must necessarily be burdensome. The bean classes can act at the translation mechanism: on the downstream interface they spit out clean XML, on the upstream interface, they spit out Matrix4d, Vector3d, etc. classes.
We would need to take some time to design the organization of these beans classes, so that they may be re-used as much as possible. For example (note the 'models' property above is actually an array), much of this representation can also be used for Model Viewer, Slide Show, and Animated cells. In fact, a cell's properties may simply be a conglomoration of standard Java bean parts -- one part gives the basic information (origin, bounds), the second the scene models, and the third any specific parts (e.g. audio 'treatments'). I'm suggesting an aggretation of Java beans here, rather than an inheritence structure necessarily.
Perhaps the biggest issue is making the system flexible enough so that authors of new cell types are not overburdened. They *should* be able to aggregate existing standard Java beans for things like origin, scene graph, etc, and then simply define a new bean class for the cell's properties that are unique to it. The code would all live within the Cell GLO package hierarchy, making changes to WFS not necessary.
This is just my latest thinking. Happy to accept comments so I can deliver a nice clean solution to all you good folks...
Jordan
|
|
|
|
|
|
|
|
|
Re: First WFS implementation
Posted:
Sep 20, 2007 12:30 PM
in response to: jslott
|
|
|
I think this proposal is on the right track. My only comment at this point is about the tag types:
> <void property="bounds">
> <void property="type"><string>Sphere</string></void>
> <void property="radius"><double>3.0</double></void>
> </void>
Is there a reason why you chose to the use the void tag for these properties?
In the following:
> <void property="translation">
> <array class="double">
> <double>5.0</double><double>-4.0</double><double>-3.0</double>
> </array>
> </void>
I think it might be a good idea to define a coordinate type attribute to specify what it is. So, instead of:
<double>5.0</double><double>-4.0</double><double>-3.0</double>
we might use:
<xcoord>5.0</xcoord>...
or perhaps:
<coord axis="x" type="double">5.0</coord>...
My preference is to make the semantics clearer to someone creating or reading the XML.
|
|
|
|
|
java.net RSS
