blender/blender

STL Import Geometry Node #122418

Merged

Hans Goudey merged 11 commits from Devashish-Lal/blender:gsoc/stl-import into main

2024-06-10 20:47:49 +02:00

Devashish Lal commented

2024-05-29 06:48:48 +02:00

Contributor

Google Summer Of Code Proposal

This PR

STL importer refactor (Mesh* exposed)
STL import Geo Node (Simple raw string input)
Error messages in Geo Node (Little warning icon above the node)
New file import nodes experimental feature flag added

[Google Summer Of Code Proposal](https://devtalk.blender.org/t/gsoc-2024-geometry-nodes-file-import-nodes/34482) This PR - [x] STL importer refactor (Mesh* exposed) - [x] STL import Geo Node (Simple raw string input) - [x] Error messages in Geo Node (Little warning icon above the node) - [x] New file import nodes experimental feature flag added

Devashish Lal added 1 commit 2024-05-29 06:49:05 +02:00

STL importer refactor b13fc9614d

Devashish Lal added 1 commit 2024-05-29 07:17:00 +02:00

Simple STL Import Geo Node 12a37a70a8

Devashish Lal added 1 commit 2024-05-29 07:40:38 +02:00

Python changes 74daf5ba50

Devashish Lal added 1 commit 2024-05-29 07:54:14 +02:00

Error messages added to stl import geo node 77d318469c

Iliya Katushenock added this to the Nodes & Physics project 2024-05-29 08:51:10 +02:00

Iliya Katushenock added the

label 2024-05-29 08:51:15 +02:00

Hans Goudey reviewed 2024-05-29 15:55:37 +02:00

Hans Goudey left a comment

Member

For error messages, import_params.reports should probably be used. That will fill the ReportList with potential errors. The items in that ReportList will need to be translated to geometry nodes error messages.

For error messages, `import_params.reports` should probably be used. That will fill the `ReportList` with potential errors. The items in that `ReportList` will need to be translated to geometry nodes error messages.

scripts/startup/bl_ui/node_add_menu_geometry.py Outdated

						
				@ -447,6 +448,14 @@ class NODE_MT_category_PRIMITIVES_MESH(Menu):

				        node_add_menu.add_node_type(layout, "GeometryNodeMeshUVSphere")

				        node_add_menu.draw_assets_for_catalog(layout, "Mesh/Primitives")

				class NODE_MT_category_IMPORT_MESH(Menu):

Hans Goudey commented

2024-05-29 15:54:19 +02:00

Member

Since import nodes won't just create curves, they may not fit well in the "Mesh" menu. How about Input/Import/?

Since import nodes won't just create curves, they may not fit well in the "Mesh" menu. How about `Input/Import/`?

Devashish Lal commented

2024-05-30 06:50:33 +02:00

Author

Contributor

Isn't nodes under Input all red color nodes? now that I think about it the Import nodes could also be a red color node. not sure how to make it red tough. I am taking a look at node_fn_input_bool.cc

Isn't nodes under Input all red color nodes? now that I think about it the Import nodes could also be a red color node. not sure how to make it red tough. I am taking a look at `node_fn_input_bool.cc`

Hans Goudey commented

2024-05-30 14:27:19 +02:00

Member

Yeah that could make sense. The red color is done with NODE_CLASS_INPUT.

Also, I'd call the menu NODE_MT_category_IMPORT, without "MESH" for the same reason.

Yeah that could make sense. The red color is done with `NODE_CLASS_INPUT`. Also, I'd call the menu `NODE_MT_category_IMPORT`, without "MESH" for the same reason.

Devashish Lal commented

2024-06-03 05:53:25 +02:00

Author

Contributor

Done

Done

Devashish-Lal marked this conversation as resolved

source/blender/nodes/geometry/nodes/node_geo_import_stl.cc Outdated

						
				@ -0,0 +24,4 @@

				  const std::string path = params.extract_input<std::string>("Path");

				  if (path.empty()) {

				    params.error_message_add(NodeWarningType::Error, TIP_("File path can't be empty"));

Hans Goudey commented

2024-05-29 15:55:18 +02:00

Member

Usually we don't add warnings for empty inputs, since they often occur in expected situations. When the path is empty, the user would just expect the node to not import anything.

Usually we don't add warnings for empty inputs, since they often occur in expected situations. When the path is empty, the user would just expect the node to not import anything.

Devashish Lal commented

2024-05-30 06:51:14 +02:00

Author

Contributor

Makes sense

Makes sense

Devashish-Lal marked this conversation as resolved

Devashish Lal added 1 commit 2024-05-30 06:57:04 +02:00

Import menu moved to input 561906d215

Iliya Katushenock reviewed 2024-05-30 10:28:39 +02:00

source/blender/nodes/geometry/nodes/node_geo_import_stl.cc

						
				@ -0,0 +14,4 @@

				static void node_declare(NodeDeclarationBuilder &b)

				{

				  b.add_input<decl::String>("Path").default_value("");

Iliya Katushenock commented

2024-05-30 10:28:39 +02:00

Member

Will mention here the thing i wrote in dock for proposal:

This is not clear how this path is related with blender instance, project file and PC of user. Does this path is relative or global?
If node tree with this node will become to asset, how path to this file will be handled? Does this means that result of node tree, modifier and asset will be different on different blender run? Might looks like violation in rule about stability, not sure.
Platform depend separators / and \ + root name C:\ and even custom extensions (if user want to open stl file with name f.geg) can not be handled in geometry nodes (and i do not like idea of added path processing nodes at least without discussion).

Before solution for all of this points will be here, i think it is better to have the generally accepted file selector property.

Will mention here the thing i wrote in dock for proposal: 1. This is not clear how this path is related with blender instance, project file and PC of user. Does this path is relative or global? 2. If node tree with this node will become to asset, how path to this file will be handled? Does this means that result of node tree, modifier and asset will be different on different blender run? Might looks like violation in rule about stability, not sure. 3. Platform depend separators `/` and `\` + root name `C:\` and even custom extensions (if user want to open stl file with name `f.geg`) can not be handled in geometry nodes (and i do not like idea of added path processing nodes at least without discussion). Before solution for all of this points will be here, i think it is better to have the generally accepted file selector property.

Devashish Lal commented

2024-05-31 08:18:45 +02:00

Author

Contributor

Yeah that’s the plant to change this to a file selector. String input for now is just for testing. I need to look at some existing nodes to see how a file selector input is setup

Yeah that’s the plant to change this to a file selector. String input for now is just for testing. I need to look at some existing nodes to see how a file selector input is setup

Iliya Katushenock commented

2024-05-31 12:31:59 +02:00

Member

Just as example you can take a look at Outout File node in compositor.

Just as example you can take a look at `Outout File` node in compositor.

Hans Goudey commented

2024-05-31 17:49:40 +02:00

Member

The idea is that the file path can be a "subtype" of the string socket type, just like "Percentage" is for float sockets. That would affect how the socket value is displayed in the UI.

The idea is that the file path can be a "subtype" of the string socket type, just like "Percentage" is for float sockets. That would affect how the socket value is displayed in the UI.

Iliya Katushenock commented

2024-05-31 18:38:25 +02:00

Member

This also might requers propagation of this fact on connected sockets (and we do not do the same thing for attribute names currently...) and have tooltips/errors/other tips related with path validation/check about file/... . But before solving of this, this can be just property\

This also might requers propagation of this fact on connected sockets (and we do not do the same thing for attribute names currently...) and have tooltips/errors/other tips related with path validation/check about file/... . But before solving of this, this can be just property\

Devashish Lal added 1 commit 2024-06-01 07:15:04 +02:00

report iteration 24f9942ea3

Devashish Lal added 1 commit 2024-06-03 18:27:13 +02:00

error reporting working e524de3d22

Devashish Lal added 1 commit 2024-06-05 09:01:16 +02:00

new import nodes experimental flag added 784e809687

Devashish Lal added 1 commit 2024-06-05 09:17:31 +02:00

Merge branch 'main' into gsoc/stl-import 090a54438b

Devashish Lal changed title from ~~WIP: STL Import Geometry Node~~ to STL Import Geometry Node

2024-06-05 20:27:23 +02:00

Iliya Katushenock reviewed 2024-06-05 20:38:43 +02:00

source/blender/nodes/geometry/nodes/node_geo_import_stl.cc Outdated

						
				@ -0,0 +31,4 @@

				    return;

				  }

				  STLImportParams p;

Iliya Katushenock commented

2024-06-05 20:31:24 +02:00

Member

p -> import_params

`p` -> `import_params`

Devashish-Lal marked this conversation as resolved

source/blender/nodes/geometry/nodes/node_geo_import_stl.cc Outdated

						
				@ -0,0 +46,4 @@

				  BKE_reports_init(&reports, RPT_STORE);

				  p.reports = &reports;

Iliya Katushenock commented

2024-06-05 20:34:37 +02:00

Member

ReportList reports;
BKE_reports_init(&reports, RPT_STORE);
p.reports = &reports;

No need for that extra spaces\

```Cpp ReportList reports; BKE_reports_init(&reports, RPT_STORE); p.reports = &reports; ``` No need for that extra spaces\

Devashish-Lal marked this conversation as resolved

source/blender/nodes/geometry/nodes/node_geo_import_stl.cc

						
				@ -0,0 +62,4 @@

				        break;

				    }

				    params.error_message_add(type, TIP_(report->message));

Iliya Katushenock commented

2024-06-05 20:36:32 +02:00

Member

~~error_message_add takes string by reference. You need to pass or static string or use some long life allocator\ (idk if message is ovned by report or no)~~
Nevermind, string will be copy by function itself\

~~`error_message_add` takes string by reference. You need to pass or static string or use some long life allocator\ (idk if `message` is ovned by report or no)~~ Nevermind, string will be copy by function itself\

mod_moder marked this conversation as resolved

source/blender/nodes/geometry/nodes/node_geo_import_stl.cc

						
				@ -0,0 +68,4 @@

				  BKE_reports_free(&reports);

				  if (mesh != nullptr) {

				    params.set_output("Mesh", GeometrySet::from_mesh(mesh));

Iliya Katushenock commented

2024-06-05 20:38:11 +02:00

Member

if (mesh == nullptr) {
  params.set_default_remaining_outputs();
}

params.set_output("Mesh", GeometrySet::from_mesh(mesh));

Plus, in case STL_import_mesh can return mesh even if there is error report (idk), should this mesh still be used?

```Cpp if (mesh == nullptr) { params.set_default_remaining_outputs(); } params.set_output("Mesh", GeometrySet::from_mesh(mesh)); ``` Plus, in case `STL_import_mesh` can return mesh even if there is error report (idk\), should this mesh still be used?

Devashish Lal commented

2024-06-06 23:01:12 +02:00

Author

Contributor

Not sure, maybe we should check for reports->list->first to be null

Not sure, maybe we should check for reports->list->first to be null

Iliya Katushenock added the

Pipeline, Assets & IO

label 2024-06-05 20:38:56 +02:00

Devashish Lal added 1 commit 2024-06-06 22:43:43 +02:00

Merge branch 'main' into gsoc/stl-import c83d2a2cc9

Devashish Lal added 1 commit 2024-06-06 22:59:50 +02:00

refactoring 1095dac5cd

Jacques Lucke commented

2024-06-10 20:35:03 +02:00

Member

LGTM. Obviously needs some more work before it can be moved out of experimental (caching, path, options?), but that can be done in main too.

LGTM. Obviously needs some more work before it can be moved out of experimental (caching, path, options?), but that can be done in `main` too.

Hans Goudey approved these changes 2024-06-10 20:36:03 +02:00

Hans Goudey referenced this issue from a commit

2024-06-10 20:47:49 +02:00

Geometry Nodes: Add STL Import Node

Hans Goudey merged commit d1455c4138 into main

2024-06-10 20:47:49 +02:00

Nika Kutsniashvili referenced this issue from a commit

2024-06-25 23:19:40 +02:00

Geometry Nodes: Add STL Import Node

Sign in to join this conversation.

No reviewers

No Label

Animation & Rigging

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Viewport & EEVEE

Virtual Reality

Asset Browser Project

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Viewport & EEVEE

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

4 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#122418

No description provided.