Java-Gaming.org    
Featured games (91)
games approved by the League of Dukes
Games in Showcase (579)
games submitted by our members
Games in WIP (500)
games currently in development
News: Read the Java Gaming Resources, or peek at the official Java tutorials
 
    Home     Help   Search   Login   Register   
Pages: [1]
  ignore  |  Print  
  Appearance.set<Atribute> - probably buggy???  (Read 1745 times)
0 Members and 1 Guest are viewing this topic.
Offline bohdan

Junior Member




Java-positive...


« Posted 2006-06-25 00:04:41 »

Hi!

I am not sure if this is already known, or if this is the bug at all, but it seems to me so, so I decide to report.
Here is description and test case as well, so you can check it fast. Would appreciate if somebody can confirm it.
------------------------------------------------
The problem is, that if you have Shape3D with the Appearance applied (let's say "myApp"), and if this Shape3D is currently invisible, just behind your field of view for instance, then changing any of the attributes of "myApp" not neccessary will have efect, well, most of the time it will, but eventualy - not.
The test case is simply bunch of quads, spread over square area with the camera located roughly at its center. The square is constantly rotating so that you see roughly half of it at a time. While in rotation the Appearance (same applied to all the quads!!!) is modified (coloring attr. for example) on regular intervals. Now have a look what you actually going to get from that....
Is it proper behaviour? BTW, I found experimentaly how to actually avoid the problem (see and try the comment at the end of code)....

It was tested with the latest Xith  _0.7.1 on JSR-231, Windows, and few different PC's (different video-cards).

1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
13  
14  
15  
16  
17  
18  
19  
20  
21  
22  
23  
24  
25  
26  
27  
28  
29  
30  
31  
32  
33  
34  
35  
36  
37  
38  
39  
40  
41  
42  
43  
44  
45  
46  
47  
48  
49  
50  
51  
52  
53  
54  
55  
56  
57  
58  
59  
60  
61  
62  
63  
64  
65  
66  
67  
68  
69  
70  
71  
72  
73  
74  
75  
76  
77  
78  
79  
80  
81  
82  
83  
84  
85  
86  
87  
88  
89  
90  
91  
92  
93  
94  
95  
96  
97  
98  
99  
100  
101  
102  
103  
104  
import java.awt.Color;

import javax.swing.JFrame;
import javax.vecmath.Color3f;
import javax.vecmath.Point3f;
import javax.vecmath.Vector3f;

import com.xith3d.render.CanvasPeer;
import com.xith3d.render.RenderPeer;
import com.xith3d.render.jsr231.RenderPeerImpl;
import com.xith3d.scenegraph.Appearance;
import com.xith3d.scenegraph.BranchGroup;
import com.xith3d.scenegraph.Canvas3D;
import com.xith3d.scenegraph.ColoringAttributes;
import com.xith3d.scenegraph.Geometry;
import com.xith3d.scenegraph.Locale;
import com.xith3d.scenegraph.Shape3D;
import com.xith3d.scenegraph.Transform3D;
import com.xith3d.scenegraph.TransformGroup;
import com.xith3d.scenegraph.View;
import com.xith3d.scenegraph.VirtualUniverse;
import com.xith3d.test.TestUtils;

public class AppearanceChangeTest {
   
    public AppearanceChangeTest() {
        JFrame frame = new JFrame("AppearanceChangeTest");
        frame.setSize(500, 500);
        //======= setting up Universe ===============
       VirtualUniverse My_Universe = new VirtualUniverse();
        Locale locale = new Locale();
        My_Universe.addLocale(locale);
        BranchGroup scRootBG  = new BranchGroup();
        locale.addBranchGraph(scRootBG);
        //------------------------------
       RenderPeer renderPeer = new RenderPeerImpl();
        CanvasPeer canvasPeer = renderPeer.
                       makeCanvas(frame.getContentPane(), 0, 0, 32, false);
        Canvas3D scrCanvas = new Canvas3D();
        scrCanvas.set3DPeer(canvasPeer);
        //------------------------------
       View scView = new View();
        scView.addCanvas3D(scrCanvas);
        My_Universe.addView(scView);
        //------ rotation group --------
       TransformGroup rotTG = new TransformGroup();
        scRootBG.addChild(rotTG);
        //======== adding many shapes3d ===============
       Appearance app = new Appearance();
        ColoringAttributes colAtt1 = new ColoringAttributes(
                new Color3f(Color.RED), 0);
        ColoringAttributes colAtt2 = new ColoringAttributes(
                new Color3f(Color.BLUE), 0);
        app.setColoringAttributes(colAtt1);
        float dL = 0.1f;
        for (float i = -5; i < 5; i++) for (float j = -5; j < 5; j++) {
            Point3f P1 = new Point3f(     i+dL,     j+dL, 0);
            Point3f P2 = new Point3f(     i+dL, j + 1-dL, 0);
            Point3f P3 = new Point3f( i + 1-dL, j + 1-dL, 0);
            Point3f P4 = new Point3f( i + 1-dL,     j+dL, 0);
            Geometry geom = TestUtils.createQuad(P1, P2, P3, P4, 1, 1);
            rotTG.addChild(new Shape3D(geom, app));
        }
        //======= running =============================
       frame.setVisible(true);
        scView.getTransform().lookAt( new Point3f (0, 2, 2),
                                      new Point3f (0, 0, 0),  
                                      new Vector3f(0, 0, 1));
        float a = 0;
        long currTime = System.currentTimeMillis();
        long lastTime = currTime;
        int count = 0; //
       Transform3D rotTF = new Transform3D();
        while (frame.isVisible()) {
            scView.renderOnce();
            //-----------------------------------
           currTime = System.currentTimeMillis();
            if (currTime - lastTime > 20) {
                lastTime = currTime;
                a += 0.03f;  if (a > 2*Math.PI) a -= 2*(float)Math.PI;  
                rotTF.rotZ(a);
                rotTG.setTransform(rotTF);
                count++;
                if (count > 50) {
                    count = 0;
                    if (app.getColoringAttributes() == colAtt1) {
                        app.setColoringAttributes(colAtt2);
                    } else {
                        app.setColoringAttributes(colAtt1);
                    }
                    //--- uncomment both lines to fix the problem ---
                   //rotTG.setLive(false);
                   //rotTG.setLive(true);
               }
            }
            //------------------------------------
       }
        System.exit(0);
    }  
   
    public static void main(String args[]) {
        new AppearanceChangeTest();
    }
}


Bohdan.
Offline bohdan

Junior Member




Java-positive...


« Reply #1 - Posted 2006-06-25 22:28:37 »

Had anybody chance to look at it?
Do I do something wrong or it is a bug?

Bohdan
Offline Amos Wenger

Senior Member




Everything's possible, but not everything's fun...


« Reply #2 - Posted 2006-06-26 19:55:41 »

I ran it and saw the things exactly how you described it.

BTW, you don't need the second setLive()

I looked at the code of SceneGraphObject and Appearance but there's nothing justifying that behavior.. Others devs, any idea ?

"Once you start working on something, don't be afraid of failure and don't abandon it. People who work sincerely are the happiest"
Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline bohdan

Junior Member




Java-positive...


« Reply #3 - Posted 2006-06-26 21:35:24 »

Thanks for reply!

I will dig into the code too and try to spot smth..

BTW, you don't need the second setLive()
aha.. Smiley alright...
Offline bohdan

Junior Member




Java-positive...


« Reply #4 - Posted 2006-06-28 18:09:10 »

Ok... I have found bug which causes this problem.

So, the story is the following.
We have banch of Shapes which share same Appearance - ok.

Strarting rendering the frame:

1) View.getRenderFrame(Canvas3D canvas)... => .... => View.renderNode(...)
2) in "View.renderNode(...)" : if (!node.isIgnoreBounds()) { ... cull the bastard Smiley))) }  - frustrum culling - this is important point, so shapes which are not visible are nod going to be render... this is very good... but!!!! next:
//----------------------------------------------------------
3) if shape is not culled => renderer.addAtom(getAtom(shape));
4) in getAtom(shape) : if (atom == null || aChanged) { atom = new ShapeAtom(shape,viewStack.getTop()); here we refreshed appearance for that shape;
5) still in "getAtom(shape)": if(aChanged) changedAppearances.add(a);!!! - again important to note!!!
here note, that all our shapes share same appearance, so basically changedAppearances list will be populated with many references of same Appearance after rendering is complete;
6) now back to "View.getRenderFrame(Canvas3D canvas)", after rendering finished: - again important for this bug:
--------------
for (int i=0;i<numChangedA;i++)
   ((Appearance)changedAppearances.get(i)).setChanged(false);   <<-- here we actually many times set to the same shared appearance changed "false".... and off cause all those shapes which actually where not rendered because they where culled - they will never get modifiyed appearance applied bacause Appearance.changed is already set to "false" Smiley))

Bohdan.


P.S. Well, I think this is difinitely that, and I think it is pretty easy to correct, ok, I make fast correction for myself, and everything is fine.
But this is not effecient correction, I think there are few things to correct. First off all, the appearance status (changed/not changed) can hold the shape itself somehow not the Appearance... Secondly, it is fairly common to share appearance so situation discussed when you iterating through the 100s same refferences - is not the best solution too and I think it is easy to correct too.

If you wish - I can correct that all (well, I paltry did), well I will try to found more efficient solution and will post it so somebody from devs can commit it.
Is that ok?
Offline Marvin Fröhlich

Senior Member




May the 4th, be with you...


« Reply #5 - Posted 2006-06-28 18:16:45 »

So it seems like you will have to use one Appearance for each Shape3D. Simply Write a method which creates it, so you won't have code duplication. Is this a performace problem for you?
Offline bohdan

Junior Member




Java-positive...


« Reply #6 - Posted 2006-06-28 18:21:02 »

Ok... but is that the solution - to have individual Appearances? The bug still exist and somebody will be caught on it like me....
And onother thing you have 1000 shapes with the same appearance, ok.. you want to change the color.... are you going to iterate through 1000 appearances?
Offline Marvin Fröhlich

Senior Member




May the 4th, be with you...


« Reply #7 - Posted 2006-06-28 18:25:22 »

Ok... but is that the solution - to have individual Appearances? The bug still exist and somebody will be caught on it like me....
And onother thing you have 1000 shapes with the same appearance, ok.. you want to change the color.... are you going to iterate through 1000 appearances?

Well this is an issue which takes time each time you change the color (or something else) but if we took away this isChanges flag, we'd loose perfromace each rendering iteration. I think it is better to loose it once, isn't it? But maybe there is just another, even better solution.
Offline bohdan

Junior Member




Java-positive...


« Reply #8 - Posted 2006-06-28 18:32:38 »

No-No!! Of cause I'm not talkin about removing the flag... I suggest, we can move that flag from Appearance to the Shape!!! no performance loss at all!!!
Offline Marvin Fröhlich

Senior Member




May the 4th, be with you...


« Reply #9 - Posted 2006-06-28 18:36:02 »

No-No!! Of cause I'm not talkin about removing the flag... I suggest, we can move that flag from Appearance to the Shape!!! no performance loss at all!!!

hmm... good point. Who is familiar with the rendering code and could check/do this?
Games published by our own members! Check 'em out!
Legends of Yore - The Casual Retro Roguelike
Offline bohdan

Junior Member




Java-positive...


« Reply #10 - Posted 2006-06-28 18:38:21 »

And actually... this flag isChanged, accordingly to the rendering scheme and definitions of Shape etc.. (ok, as for me personaly) - is actually more "applicable" really to Shape, not to the Appearance itself, this is more logically I think... What you say?
Offline bohdan

Junior Member




Java-positive...


« Reply #11 - Posted 2006-06-28 18:57:02 »

Look, it is even looking nicer: rather than having flag in appearance which indicates that it was changed (even though, having it set you not going to do anything with that appearance, you have changes committed straigth away...) - it will sit in Shape and indicates that its appearance was changed and needs to be "reapplied" - sounds better!!!

And, more to it, having that done - we don't need that "changedAppearances" list at all!!! because as soon as we "reapplied" appearance for shape we reseting its flag (shape's flag) at the same time - nothing needs to be stored for latter "clearance"!! So actually - it even increases performance, well at least little bit + code would be more consistent and clear.
Offline Amos Wenger

Senior Member




Everything's possible, but not everything's fun...


« Reply #12 - Posted 2006-06-29 15:49:09 »

Look, it is even looking nicer: rather than having flag in appearance which indicates that it was changed (even though, having it set you not going to do anything with that appearance, you have changes committed straigth away...) - it will sit in Shape and indicates that its appearance was changed and needs to be "reapplied" - sounds better!!!

And, more to it, having that done - we don't need that "changedAppearances" list at all!!! because as soon as we "reapplied" appearance for shape we reseting its flag (shape's flag) at the same time - nothing needs to be stored for latter "clearance"!! So actually - it even increases performance, well at least little bit + code would be more consistent and clear.
Definitely go for it. I'll commit it.

"Once you start working on something, don't be afraid of failure and don't abandon it. People who work sincerely are the happiest"
Offline bohdan

Junior Member




Java-positive...


« Reply #13 - Posted 2006-06-29 19:04:52 »

Ok, thanks Smiley

So I'll post the changes I suggest shortly (today hopefully if I have a time).

Bohdan.
Offline bohdan

Junior Member




Java-positive...


« Reply #14 - Posted 2006-06-30 04:31:24 »

Ok, here are the changes:

Since "changed" flag is actually the flag of super class (NodeComponent), I didn't really "move" it (to make sure I will not brake anything), but the code below will act same way really.

Appearance will have now its unique indentifier "changeID" that is updated each time (see getAtom(...)) any of the Appearance attributes change. Change identifier for Appearance is updated only once per frame rendering (if appearance was changed) even if appearance is shared by many Shapes3D.

At the same time each Shape3D has its own "appearance change identifier" which is to be compared to appearance's one. If it is not equal - appearance would be reapplied, otherwise - not. If change was applied corresponding identifiers become equal. This allows considering Appearance changes on per Shape3D basis rather than on per Appearance basis as in current implementation, and still having Appearance sharing in place.

Please have a look at the code and comment if you feel something is wrong. There should be no compatibility problems, since no methods which may be potentially used outside of frame rendering procedure were modified. The setChanged(..) method is unafected - you still can used it (if you ever need it really...) and it will work in this scheme with no problems too. I don't see any performance loss here, frequent/excessive memory allocations or extra garbage collection... (and changeID is never set to null, except the only case - when Appearance is set to null).

So, the actually code:

In com.xith3d.scenegraph.Appearance (nothing to modify, just add the following):
1  
2  
3  
4  
5  
6  
7  
8  
9  
+ private Object changeID = null;
+    
+ protected final Object verifyChange()  {
+       if (isChanged())  {
+             changeID = new Object();
+             setChanged(false);
+       }
+       return changeID;
+ }



In com.xith3d.scenegraph.Shape3D (nothing to modify, just add the following):
1  
2  
3  
4  
5  
6  
7  
8  
9  
10  
11  
12  
13  
14  
15  
+ private Object appChangeID = null;
+  
+ protected boolean verifyAppChange()  {
+     if (appearance != null)  {
+           Object change_id = appearance.verifyChange();
+           if (change_id != appChangeID)  {
+                 appChangeID = change_id;
+                 return true;
+           }
+     } else if (appChangeID != null) {
+            appChangeID = null;
+            return true;
+     }
+     return false;
+ }



In com.xith3d.scenegraph.View.getAtom(Shape3D shape) :
1  
2  
3  
4  
5  
- Appearance a = shape.getAppearance();
- boolean aChanged = (a != null && a.isChanged());
- if (atom == null || aChanged) {
+ if (atom == null || shape.verifyAppChange()) {
- if(aChanged) changedAppearances.add(a);



In com.xith3d.scenegraph.View.getRenderFrame(Canvas3D canvas) (remove, as not needed anymore)
1  
2  
3  
4  
- final int numChangedA = changedAppearances.size();
- for (int i=0;i<numChangedA;i++)
-     ((Appearance)changedAppearances.get(i)).setChanged(false);
- changedAppearances.clear();



In com.xith3d.scenegraph.View: (remove declaration as unused)
1  
- ArrayList changedAppearances = new ArrayList();



Bohdan.
Offline Amos Wenger

Senior Member




Everything's possible, but not everything's fun...


« Reply #15 - Posted 2006-06-30 16:49:17 »

Committed.

"Once you start working on something, don't be afraid of failure and don't abandon it. People who work sincerely are the happiest"
Offline bohdan

Junior Member




Java-positive...


« Reply #16 - Posted 2006-06-30 18:16:07 »

Thanks!!! 
Pages: [1]
  ignore  |  Print  
 
 
You cannot reply to this message, because it is very, very old.

 

Add your game by posting it in the WIP section,
or publish it in Showcase.

The first screenshot will be displayed as a thumbnail.

xsi3rr4x (42 views)
2014-04-15 18:08:23

BurntPizza (38 views)
2014-04-15 03:46:01

UprightPath (54 views)
2014-04-14 17:39:50

UprightPath (36 views)
2014-04-14 17:35:47

Porlus (53 views)
2014-04-14 15:48:38

tom_mai78101 (75 views)
2014-04-10 04:04:31

BurntPizza (134 views)
2014-04-08 23:06:04

tom_mai78101 (234 views)
2014-04-05 13:34:39

trollwarrior1 (195 views)
2014-04-04 12:06:45

CJLetsGame (203 views)
2014-04-01 02:16:10
List of Learning Resources
by SHC
2014-04-18 03:17:39

List of Learning Resources
by Longarmx
2014-04-08 03:14:44

Good Examples
by matheus23
2014-04-05 13:51:37

Good Examples
by Grunnt
2014-04-03 15:48:46

Good Examples
by Grunnt
2014-04-03 15:48:37

Good Examples
by matheus23
2014-04-01 18:40:51

Good Examples
by matheus23
2014-04-01 18:40:34

Anonymous/Local/Inner class gotchas
by Roquen
2014-03-11 15:22:30
java-gaming.org is not responsible for the content posted by its members, including references to external websites, and other references that may or may not have a relation with our primarily gaming and game production oriented community. inquiries and complaints can be sent via email to the info‑account of the company managing the website of java‑gaming.org
Powered by MySQL Powered by PHP Powered by SMF 1.1.18 | SMF © 2013, Simple Machines | Managed by Enhanced Four Valid XHTML 1.0! Valid CSS!