Feature: 3D Renderer and Preview Export of Thumbnails #87

rahelarnold98 · 2024-07-12T09:19:02Z

This pull request integrates a 3D model renderer and introduces a 3D model preview exporter.

The renderer, originally developed by @net-cscience-raphael for Cineast using LWJGL, retains its core functionality while the texture model has been translated to Kotlin for enhanced integration. This updated version replaces the previous implementation in the vitrivr-engine-core module.

In addition, new functionality has been added to create previews of 3D models in the vitrivr-engine-module-m3d/src/main/kotlin/org/vitrivr/engine/model3d/ModelPreviewExporter.kt. By specifying settings in the schema-config.json file, users can export either GIFs or JPGs that showcase 3D models. GIFs allow for a customizable number of views capturing different angles of the 3D model, while the JPG version provides four predefined views. To ensure correct formatting of exported previe ws, the mimeType of the DiskResolver must be adjusted in the config file.

Overview of new Functionality:

Integration of a 3D model renderer
Export of GIF previews for 3D models
Export of JPG previews for 3D models

…lows

ppanopticon · 2024-07-15T07:28:16Z

Thanks for the PR.

In order to be able to review this it would be nice if you could add some minimal working example / documentation snippet as to how this can be used and tested.

Furthermore, I was wondering if it would be possible to have a minimal UnitTest that makes sure that rendering works. See PR #84 for further reference.

ppanopticon · 2024-07-26T08:03:45Z

In addition to the detailed comments and the previous remark, I have two more fundamental issue with this PR and the way the M3D module is setup:

I don't think it's particularly wise to add Java code to a Kotlin project, unless there is a good reason to (which I don't see).
I was wondering if the JOML dependency in the vitrivr-engine-core module could be removed by using our own datatypes and convert them, once Vector3f are actually needed.

In its current state, the PR seems to violate some basic principles of the project (e.g., the ability to include or omit certain modules). Furthermore, it seems to be a bit messy since it's littered with empty classes and deprecated code.

lucaro

This pull request is clearly not ready. It contains lots of Java code that has no place in a kotlin code base (especially not under src/main/kotlin), including various deprecated methods, interfaces, and classes. The rendering mechanism is needlessly complicated and does not adhere to any of the architectural conventions we introduced in this project. The way this is now, this looks not maintainable. Some substantial rework is required before this can be merged.

lucaro · 2024-07-12T09:28:49Z

vitrivr-engine-core/src/main/kotlin/org/vitrivr/engine/core/model/mesh/texturemodel/Model.kt

+ * position and scale the model in the scene. The [Material] objects are used to define the
+ * appearance of the model.
+ */
+data class Model(


This, again, is a very generic name, as it could 'model' anything. I suggest renaming it to something more descriptive.

Renamed model to model3d to specify that a 3d model is used

lucaro · 2024-07-26T13:26:36Z

...e-core/src/main/kotlin/org/vitrivr/engine/core/model/content/factory/CachedContentFactory.kt

@@ -117,7 +117,7 @@ class CachedContentFactory : ContentFactoriesFactory {
            return content
        }

-        override fun newMeshContent(model3D: Model3D): Model3DContent {
+        override fun newMeshContent(model: Model): ModelContent {
            check(!this.closed) { "CachedContentFactory has been closed." }
            TODO()


This should be properly implemented before merging this PR.

lucaro · 2024-07-26T13:27:37Z

...vr-engine-core/src/main/kotlin/org/vitrivr/engine/core/model/content/element/ModelContent.kt


 /**
 * A 3D [ContentElement].
 *
 * @author Rahel Arnold
 * @version 1.0.0
 */
-interface Model3DContent: ContentElement<Model3D>{
-    /** The [ContentType] of a [Model3DContent] is always [ContentType.MESH]. */
+interface ModelContent: ContentElement<Model>{


ModelContent is very generic; the previous name was more descriptive. I'd change it back.

lucaro · 2024-07-26T13:29:25Z

vitrivr-engine-core/src/main/kotlin/org/vitrivr/engine/core/model/mesh/texturemodel/Entity.kt

-    private val modelId: String
-) {
-    private val LOGGER = LogManager.getLogger()
+class Entity(val id: String, val modelId: String) {


This, again, is a very generic name. I'm sure there is a more descriptive one.

lucaro · 2024-07-26T13:30:39Z

vitrivr-engine-core/src/main/kotlin/org/vitrivr/engine/core/model/mesh/texturemodel/IModel.kt


 import org.joml.Vector3f

-/**
- *
- */
 interface IModel {


I'm personally not a big fan of the I-prefix naming scheme for interfaces. It usually indicates that there is another thing that should have a more specific name.

lucaro · 2024-07-26T14:03:54Z

...c/main/java/org/vitrivr/engine/model3d/data/render/lwjgl/util/datatype/VariantException.java

+ * Used to indicate that a key is not valid.
+ */
+@SuppressWarnings("unused")
+public class VariantException extends RuntimeException {


lucaro · 2024-07-26T14:04:31Z

.../src/main/java/org/vitrivr/engine/model3d/data/render/lwjgl/util/fsm/abstractworker/Job.java

+ * If a job is a CONTROL job it contains a command
+ * {@link  JobControlCommand}
+ */
+public abstract class Job {


I don't understand why this is needed

lucaro · 2024-07-26T14:05:27Z

.../engine/model3d/data/render/lwjgl/util/fsm/abstractworker/StateProviderAnnotationParser.java

+ * This class is used to parse the annotations of a state provider
+ * @see StateProviderAnnotationParser#runTransitionMethods(Object, State, State, Transition, Variant)
+ */
+public class StateProviderAnnotationParser {


This is very much not the kotlin way of thigs

lucaro · 2024-07-26T14:05:53Z

...ava/org/vitrivr/engine/model3d/data/render/lwjgl/util/fsm/controller/FiniteStateMachine.java

+ * @see <a href="https://stackoverflow.com/questions/5923767/simple-state-machine-example-in-c">
+ * https://stackoverflow.com/questions/5923767/simple-state-machine-example-in-c/a>
+ */
+public class FiniteStateMachine {


lucaro · 2024-07-26T14:07:36Z

vitrivr-engine-server/src/main/kotlin/org/vitrivr/engine/server/Main.kt

+    val renderWorker = RenderWorker(renderJobQueue)
+
+    /* Run RenderWorker on the main thread. */
+    renderWorker.run()


rahelarnold98 · 2024-07-29T12:58:06Z

Thank you for the detailed review @lucaro. I will work through the comments and make the adjustments.

Nevertheless, I think we should discuss whether the complete rendering logic that uses LWJGL must be translated to Kotlin or if we can include this independent Java code. The code was directly copied from Cineast, so the finite state machine was implemented like this before.

Furthermore, I have not found a Java class in src/main/kotlin. Please point them out. For me, all Java code is located under src/main/java.

@ppanopticon Thank you as well for your feedback. I will add an example and unit tests and rewrite some parts to remove the JOML dependency in the core module.

Signed-off-by: Ralph Gasser <[email protected]>

lucaro · 2024-07-29T13:25:36Z

I'm aware that this was implemented like this in Cineast. It was over-engineered there (as I already pointed out back then), and it is over-engineered here. Here, it's even worse since Cineast had already collected some bloat over the years, so the addition was relatively not as bad. I'm also opposed to supporting multiple languages in a project with no clear benefit. This just complicates maintenance and reduces readability in the long run. Translating the Java code to Kotlin is straightforward and even (mostly) automated by Intellij. While translating this (or, mostly, having it automatically translated), there will also be room to get rid of some inherent boilerplate code that is just not required in Kotlin.
You are right about the package structure, the Java classes all appear to be in src/main/java, my bad.

ppanopticon · 2024-07-29T13:31:41Z

Okay so I have taken another look at this, especially how this can be integrated without interfering with the flow of the integrating application. Having spent half a day on this I can conclude, that the only way we can use the rendering logic without hooking it into the application's main thread is by externalising it into a different process.

For this purpose, I have created the Renderer and the ExternalRenderer implementation, which are basically two sides of a coin: The Renderer is a small Java application (basically a main() method) that reads RenderRequests from a an ObjectInputStream, processes them and writes them to the ObjectOutputStream. The ExternalRenderer spawns such a process, generates and sendsRenderRequests and processes incoming RenderResponses. The very simple example seems to work. However, there is an issue in ModelPreviewExporter.renderPreviewJPEG that makes the unit test fail (I'll add a comment).

Having said that: I agree with @lucaro that the entire rendering logic (i.e., this state-machine stuff) is way too complicated for solving a simple problem such as rendering a few previews. I really wonder, if this can't be simplified. To me, the fact that the entire state-machine logic in Worker is basically a Runnable (which is typically supposed to run in a different thread) and that GLFW must be executed on the main thread is a clear sign that the solution and the problem do not fit together.

Signed-off-by: Ralph Gasser <[email protected]>

rahelarnold98 · 2024-08-16T08:43:21Z

The JOML dependency in the vitriol-engine-core module is now removed.
Concerning documentation, I will add the instructions in the Wiki.

rahelarnold98 · 2024-08-16T09:57:56Z

An example of how the configuration should look like to create previews can already be found in the Wiki: https://github.com/vitrivr/vitrivr-engine/wiki/Example#3d-model-pipeline

Signed-off-by: Ralph Gasser <[email protected]>

…to work). Signed-off-by: Ralph Gasser <[email protected]>

Signed-off-by: Ralph Gasser <[email protected]>

…ess. Signed-off-by: Ralph Gasser <[email protected]>

Signed-off-by: Ralph Gasser <[email protected]>

…h current Cottontail DB driver version.

ppanopticon · 2024-07-26T07:40:51Z

vitrivr-engine-server/src/main/kotlin/org/vitrivr/engine/server/Main.kt

+    }
+
+    /* Initialize RenderWorker on the main thread. */
+    val renderJobQueue = LinkedBlockingDeque<RenderJob>()


Adding this to the vitrivr-engine-server like this goes against the modularity idea of vitrivr-engine (i.e., the ability to actually omit this module in a concrete project).

ppanopticon · 2024-07-26T07:44:18Z

vitrivr-engine-index/build.gradle

@@ -7,6 +7,7 @@ plugins {
 dependencies {
    /* vitrivr core dependency. */
    api project(':vitrivr-engine-core')
+    api project(':vitrivr-engine-module-m3d')


The vitrivr-engine-index module must no depend on vitrivr-engine-module-m3d. This makes no sense from an architectural perspective.

If anything, the vitrivr-engine-module-m3d can be added to the vitrivr-engine-module-server for testing purposes only.

ppanopticon · 2024-07-26T07:49:54Z

vitrivr-engine-module-m3d/src/main/java/org/vitrivr/engine/model3d/data/Pair.java

+
+import java.util.Objects;
+
+public class Pair<K, V> {


If this was Kotlin code, then we wouldn't need to have this class.

ppanopticon · 2024-07-26T07:54:21Z

...r-engine-module-m3d/src/main/java/org/vitrivr/engine/model3d/data/render/lwjgl/Renderer.java

@@ -0,0 +1,2 @@
+package org.vitrivr.engine.model3d.data.render.lwjgl;public class Renderer {


What is the purpose of this class?

ppanopticon · 2024-07-26T07:54:40Z

vitrivr-engine-module-m3d/src/main/java/org/vitrivr/engine/model3d/data/render/Renderer.java

+ * - Rendering of single Mesh or VoxelGrid - Free positioning of the camera in terms of either cartesian or polar coordinate - Snapshot of the rendered image can be obtained at any time.
+ *  @deprecated  This interface is deprecated it can be simplified if the old JOGL renderer is removed.
+ */
+@Deprecated


Why add this to the project if it is already deprecated?

...vr-engine-module-m3d/src/main/kotlin/org/vitrivr/engine/model3d/renderer/ExternalRenderer.kt

rahelarnold98 · 2024-08-26T08:53:41Z

@lucaro This pull request is ready to merge from my side. Could you please review it once more?

lucaro · 2024-08-26T14:04:48Z

If @ppanopticon gave his approval, it's good enough for me 😉

rahelarnold98 added 10 commits July 8, 2024 19:24

init rendering work from @net-cscience-raphael (see Cineast)

9cc66fe

rendering of 3d models with embedded textures completed

9fa71ca

3D rendering and extracting of a preview finally works🥳, clean up fol…

25a279c

…lows

deleted duplicated and no longer used files

f362c99

more cleanup

9dfa0a3

deleted unused files

8a21952

minor cleanup changes

09a75c6

3d model preview can now be a gif of different views

ccac2de

removed print statements used while debuggung

8678d62

rendering window is hidden

b2d3e55

rahelarnold98 requested review from ppanopticon and net-cscience-raphael July 12, 2024 09:19

rahelarnold98 marked this pull request as ready for review July 12, 2024 09:19

rahelarnold98 self-assigned this Jul 12, 2024

ppanopticon changed the title ~~Feature: 3D Renderer and Preveiw Export of Thumbnails~~ Feature: 3D Renderer and Preview Export of Thumbnails Jul 26, 2024

ppanopticon requested a review from lucaro July 26, 2024 08:04

lucaro requested changes Jul 26, 2024

View reviewed changes

rahelarnold98 added 5 commits July 29, 2024 12:02

renamed model to model3d to be more specific

2b15d7d

renamed ModelContent to Model3DContent to be more specific

237c538

removed deprecated and unused functions from core.model.mesh

f50b53d

changed OptimizerOptions to class

a1ee346

removed empty class

8714eca

ppanopticon added 2 commits July 29, 2024 15:12

Adds facility for external rendering.

335b167

Signed-off-by: Ralph Gasser <[email protected]>

Adds facility for external rendering.

170f676

Signed-off-by: Ralph Gasser <[email protected]>

Fixes combination of four images in ModelPreviewExporter.

641453f

Signed-off-by: Ralph Gasser <[email protected]>

rahelarnold98 added 2 commits August 7, 2024 14:02

Camera is now a data class

7e8d974

removed joml dependency in core module

70f3a36

ppanopticon requested review from lucaro and removed request for net-cscience-raphael August 19, 2024 07:27

rahelarnold98 and others added 16 commits August 22, 2024 13:52

merged dev in feature branch

2a28b39

Minor optimisation as proposed by @lucaro.

7f276a2

Signed-off-by: Ralph Gasser <[email protected]>

Attempt at better error reporting.

8aecbef

Signed-off-by: Ralph Gasser <[email protected]>

The -XstartOnFirstThread is only used on MacOS.

16affa6

Signed-off-by: Ralph Gasser <[email protected]>

Improves error reporting.

ad07a8f

Signed-off-by: Ralph Gasser <[email protected]>

GitHub action script now creates a screen (in an attempt to get GLFW …

10042ba

…to work). Signed-off-by: Ralph Gasser <[email protected]>

Removes logging.

b02197e

Signed-off-by: Ralph Gasser <[email protected]>

Removes change, since it doesn't seem to work.

6c47ac2

Signed-off-by: Ralph Gasser <[email protected]>

Dependency clean-up.

50e609d

Signed-off-by: Ralph Gasser <[email protected]>

Adds display to ubuntu container and runs tests with display.

cba45f5

Signed-off-by: Ralph Gasser <[email protected]>

Adds test reporting to CI/CD.

6961a83

Signed-off-by: Ralph Gasser <[email protected]>

Merge branch 'dev' into feature/3d-preview

7a14cee

DISPLAY=:1 is now integrated into ExternalRenderer invocation of proc…

1d719a6

…ess. Signed-off-by: Ralph Gasser <[email protected]>

Fix.

eaa3821

Signed-off-by: Ralph Gasser <[email protected]>

Merged

02dfef1

Downgrades Protobuf dependency, since it seems to be incompatible wit…

1247b9c

…h current Cottontail DB driver version.

ppanopticon added the enhancement New feature or request label Aug 26, 2024

ppanopticon requested review from ppanopticon and removed request for ppanopticon August 26, 2024 08:41

ppanopticon approved these changes Aug 26, 2024

View reviewed changes

rahelarnold98 merged commit 9fee7e5 into dev Aug 27, 2024
2 checks passed

Spiess deleted the feature/3d-preview branch October 18, 2024 06:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: 3D Renderer and Preview Export of Thumbnails #87

Feature: 3D Renderer and Preview Export of Thumbnails #87

rahelarnold98 commented Jul 12, 2024

ppanopticon commented Jul 15, 2024

ppanopticon commented Jul 26, 2024 •

edited

Loading

lucaro left a comment

lucaro Jul 12, 2024

rahelarnold98 Jul 29, 2024

lucaro Jul 26, 2024

lucaro Jul 26, 2024

rahelarnold98 Jul 29, 2024

lucaro Jul 26, 2024

lucaro Jul 26, 2024

lucaro Jul 26, 2024

lucaro Jul 26, 2024

lucaro Jul 26, 2024

lucaro Jul 26, 2024

lucaro Jul 26, 2024

rahelarnold98 commented Jul 29, 2024

lucaro commented Jul 29, 2024

ppanopticon commented Jul 29, 2024 •

edited

Loading

rahelarnold98 commented Aug 16, 2024

rahelarnold98 commented Aug 16, 2024

ppanopticon Jul 26, 2024

ppanopticon Jul 26, 2024

ppanopticon Jul 26, 2024

ppanopticon Jul 26, 2024

ppanopticon Jul 26, 2024

rahelarnold98 commented Aug 26, 2024

lucaro commented Aug 26, 2024

		@@ -0,0 +1,2 @@
		package org.vitrivr.engine.model3d.data.render.lwjgl;public class Renderer {

Feature: 3D Renderer and Preview Export of Thumbnails #87

Feature: 3D Renderer and Preview Export of Thumbnails #87

Conversation

rahelarnold98 commented Jul 12, 2024

ppanopticon commented Jul 15, 2024

ppanopticon commented Jul 26, 2024 • edited Loading

lucaro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahelarnold98 commented Jul 29, 2024

lucaro commented Jul 29, 2024

ppanopticon commented Jul 29, 2024 • edited Loading

rahelarnold98 commented Aug 16, 2024

rahelarnold98 commented Aug 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rahelarnold98 commented Aug 26, 2024

lucaro commented Aug 26, 2024

ppanopticon commented Jul 26, 2024 •

edited

Loading

ppanopticon commented Jul 29, 2024 •

edited

Loading