xeolabs / xeogl

A WebGL-based 3D engine for technical visualization. Not actively maintained.
http://xeogl.org
Other
1.15k stars 264 forks source link

bug in xeogl.clear, added several features #276

Closed stefHin closed 1 year ago

stefHin commented 5 years ago

hi

As I'm using Xeogl in my software, I had to make some changes to it to make it work for my purposes. Some of them you may consider useful, but I only edited the built xeogl.js file (so adding changes to src files you would have to do manually)

1: when calling ceogl.clear(), it crashes (NullPointerException). I added a check if the object of the accessed property is null, which prevents crashing, but I'm not sure if everything is destroyed correctly (I suppose not as the heap keeps growing about 3 MB every time I create a scene and load the model and then destroy it again). I am not using the default scene but create one with new xeogl.Scene(...).

2: CameraControl offers no interface to set zoom, pan and rotate factor manually. I added those and use a slider in dat.gui of my software to let the user modify it while viewing a model. This is necessary because those rates should work more precisely when navigating through a building in first-person mode than when viewing the building from outside.

3: Mouse events are already implemented, but the event you get is not very useful when you want to distinguish between left, right and middle mouse key. I added a mousewheel listener for my purposes, but I suppose it would be better to simly make the event parameter contain more information.

4: Using wrong canvas width as height in pin.js, probably leads to wrong visibility checking.

I also noticed that the annotations occludable feature only works when initially set to true. Later disabling it works (and activating it again as well), but when initially set to false and then set to true, the annotations are never visible (no matter which position in space). Also the visibility checking for borders of the canvas is inactive when occludable is initially set to false (not sure if its activated when changing occludable later) - not really a bug as the annotations are simply shifted out of the screen then, but I just noticed it.

stefHin commented 5 years ago

added bugfix for "flying annotations": when viewing annotations inside a building with the firstPersonView active, it appears that the annotations are somehow mirrored at the user's viewpoint. So you sometimes have 2 annotation, one of them flying kind of randomly in the air... Checking if tempVec4b[2]<0 and in that case setting one of the coordinates outside the user's screen boundaries fixes that.

xeolabs commented 5 years ago

Thanks @stefHin - I won't merge this PR, instead I'll make these fixes in the codebase "manually", using your PR as a guide.

I just fixed xeogl.clear() in master: https://github.com/xeolabs/xeogl/commit/7ec555f2ef770e106c389c4b15142eda50551290

I also did that fix in v0.9, the ES6 conversion branch.

I made a ticket for that bug here: https://github.com/xeolabs/xeogl/issues/278