lbartworks / openvgal

Virtual 3D gallery for art showcase. Based on Babylon.js
MIT License
20 stars 7 forks source link

Some code correction requirements #2

Closed Flow-R-Ian closed 6 months ago

Flow-R-Ian commented 8 months ago

Greetings!

First of all thank you very much for this awesome software to be available as Open Source product. I searched the Internet for something like this to show photos in a gallery-style way.

Now let me write my experience in an issue report: For this product test I use Windows 10 + Firefox + Apache webserver for win64 - and followed the instruction ("4. Steps to create a gallery") as of 20. Jan. 2024.

#####################################

4.0 Main step: Preparation

  1. Organizing photos in various gallery* subdirectories, keeping in mind the <50 images / dir rules of thumb mentioned later in the instructions - Check.

-----------------

  1. create a csv file (building.csv) in the python subdirectory -- Check.

-----------------

  1. Install local webserver (apache) - Check.

-----------------

  1. Install jupyter notebooks --- and on windows painfully learn how to set it to use the same root dir as the Apache. - Check.

-----------------

  1. Download the VGAL repo. - Check,

-----------------

  1. (actually the 6. step is marked as 7. in the instructions) Galleries are already there. - Check.

-----------------

  1. Run the jupyter notebook and load the notebook VR_gallery.ipynb... Well: there are some concerns here.

The default values in the VR_gallery.ipynb for: "building[\"Technical\"][\"ambientLight\"]=10\n", "building[\"Technical\"][\"pointLight\"]=50\n",

Are too much. ambientLight could be 1 while pointlight should be 20.

Also if the image files inside the gallery directories have the extension with capital letters for example .JPG the script simply gets stuck and Jupyter Notebook does not say at which file. It is good to have the "Handle.exe" tool for windows (something like the "fuser" on Linux ) to find the file where the script stands at, having the PID holding the file - as well as cygwin for bash-like shell commands and iterations for faster file operation thoughout the gallery directories.

Another point here is the aspect ratio of images (height / width): the script seems like tries to guess (?!) the ascect ratio however in some cases they have to be corrected manually in the building.json file - which is a pain in the A job when there are hundreds of images and dozens of galleries.

Of course in case of "dozens of galleries" with <50 images each - the room sizing must be set manually as a trial and error method until the right measure has been found. (the gallery viewer is a good way to test it later on).

Hint for users: Also the source csv file (building.csv) should be plain, therefore it is not good to edit them - for example - in Excel so there should be no apostrophes present. The building.json file will be in the root dir of the apache (by default: [Drive]:\Apache24\htdocs directory). It is advised to keep it there and to copy the styles.json there as well. Also copy the new building.json into the vgal subdirectory.

-----------------

4.1 Main step: Hall Builder.

  1. In "/vgal/hall_builder/room_processor.html" I guess the styles.json is missing from the constants. It calls the "./room_builder_aux.js" java script right at the beginning.

In Firefox the room_builder_aux.js script run on an error stating that "floor_material" is not defined. It is defined, and the script seems good: floor_material = materials[data.chosenStyles.chosenFloorStyle];

(Also to mention that the "vgal/blender/scrypt.py" overrides the material attributes.)

When I manually declare the variable to match the information contained in styles.json file as floor_material = "Wooden"; The next error is that the rb function is not defined in room_processor.html. As I am not too familiar with the concept (beginner level in python and in Java) so I leave this as it is.

Hall Builder failed for me (this is step 10.) on Windows 10 + Firefox (tried it in Edge and IE compatibility mode as well - No use).

-----------------

  1. There is no step 9.

-----------------

  1. Yep, Hall Builder failed.

-----------------

  1. No downladed room files due to error.

-----------------

  1. There is no step 12.

-----------------

  1. Probably cannot do without generated files.

-----------------

4.2 Main Step: Hall template --- I skipped this during my tests.

-----------------

4.3 Main step: On the fly creation (no glb file) "http://localhost/vgal/gallery_viewer/virtual_gallery.html"

The first problem is with the double quites - they shall be single apostrophes: const asset_location="/"; const config_file_name="/building.json"; const styles_file_name="/styles.json" const materials_folder='/vgal/materials'; const hallspics_prefix= '/vgal';

About const hallspics_prefix= '/vgal'; should this point to /vgal/python ?

Otherwise it works like charm if the styles.json is also in the root directory of the webserver.

The only issue is that the internal room doors are not working as a link back to the main hall (root) so if I want to get back into the corridor I have to refresh the page.

Plus another big issue after the GalleryViewer works: when I want to export the room(s) to GLB file(s) inside the "GalleryViewer" (babylon.js) the webpage informs me about the Export being in progress... and nothing happens. There the Firefox web console gives the following information:

BJS - [timestamp]: GLTF2Export is not available. Make sure to load the serializers library

Uncaught TypeError: jl.GLTF2Export is undefined exportGLTF toolsTabComponent.tsx:286 onClick toolsTabComponent.tsx:501 onClick buttonLineComponent.tsx:20 React 11 unstable_runWithPriority scheduler.production.min.js:18 React 3

The coding reference of Firefox shows the following: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Unexpected_type?utm_source=mozilla&utm_medium=firefox-console-errors&utm_campaign=default

-----------------

So overall the project is very exciting and satisfying as it works like charm in GalleryViewer and needs some manual preparation to make it totally custom. The textures and sizes etc. are also manually customizable and is a good feature.

Definitely worth to continue developing it, keep up the good work!

Sincerely, Flow-R-Ian

lbartworks commented 8 months ago

Hi Flow-R-Ian Thanks so much for the thorough review !! and thanks so much for the nice and encouraging words ❤️ Another user warned me about the light issue. I had been testing lightmaps (that require much higher illumination) and the setting remained. I have already fixed it.

I will go tomorrow through the rest. Some of them are related to the fact that the instructions were quite thorough for v0.1. Since then I have added many features but I have prioritized development for updated documentation. For example the hall builder will be discontinued. The on-the-fly generation does the job in a much more efficient way. A glb file makes sense if it is generated by a 3D tool (blender for ex). As a fallback solution could be generated from the debug menu as you tried. Updating the documentation is in my todo list. I can also create a video.

I have some blender templates and python code to read the json file. The looking is much better but you gotta know blender, which is not trivial.

lbartworks commented 8 months ago

I went through the list.

I need more information for these:

Flow-R-Ian commented 8 months ago

Hi again,

Thank you for the fast reaction and code corrections. Just cheked it again:

With the door (leading back to the root corridor from a gallery) the window size has not changed. Inside a gallery it just displays a white rectangle at the place of the door with a label: "Entrance". The item seems not active. There is an error in the browser console:

Uncaught (in promise) TypeError: item_names[j] is undefined rb room_builder_aux.js:392 galleryManager virtual_gallery.html:454 execute directActions.ts:511 _executeCurrent action.ts:173 processTrigger actionManager.ts:353 _processPointerUp scene.inputManager.ts:499 _onPointerUp scene.inputManager.ts:979 _initClickEvent scene.inputManager.ts:669 _onPointerUp scene.inputManager.ts:908 attachControl scene.inputManager.ts:1044 notifyObservers observable.ts:393 _onInputChanged deviceSourceManager.ts:165 o internalDeviceSourceManager.ts:79 _pointerUpEvent webDeviceInputSystem.ts:605

##################

As for the aspect ratio I checked the creation of the building.json file again and caught an example of a portait direction image getting identified as being in "Landscape" direction. Image_aspect_ratio_error

But that is all. Hope they require just a minor correction. Sincerely, Flow-R-Ian

lbartworks commented 8 months ago

Hi ! On the second one what would help me is an image (the actual file) showing the problem, but I guess I understand the issue. It must be related to the fact that sometimes the exif information (the metadata) in the image saves the orientation information. All modern cameras include it. Most viewers will read it and rotate automatically the image for you. The script in python could read that metadata and correct the width/height ratio if a rotation is neeed. In the meantime you can enforce the rotation of the EXIF orientation field with many tools (https://linux.die.net/man/1/exiftran for example, there must be many). They will losslessly rotate the image and fix the issue.

On the second one it is a bit tricky to trouble shoot just with the error. What you might have forgotten (actually I think I forgot to include it in the documentation) is to include the in the csv file (#items N) one item for the door to the main entrance, normally in the north wall. What this means is that if you have 20 images in a gallery and you distribute 5+5+5+5 in the four walls, that will miss the door. In such case the number of items should be 6 +5 +5 +5. If that would not work, send me the building json and I can try to dig further.

I will probably work directly on v1 with major updates in automation and flexibility to use materials.

lbartworks commented 7 months ago

Version 0.99 is almost ready fixing these issues and adding features. I need to write the documentation

Flow-R-Ian commented 7 months ago

Awesome, thank you.

lbartworks commented 6 months ago

The just uploaded version brings a lot of improvements, including metadata orientation detection. This closes, the list of issues you reported. Enjoy