Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Godot 3.0 primitives as resources for use with MeshInstance #8661

Merged
merged 1 commit into from
Jun 16, 2017

Conversation

BastiaanOlij
Copy link
Contributor

I've ported PR #8578 to Godot 3 and as suggested by @reduz changed the implementation to resources subclassed from Mesh instead of Nodes.

There are a few small fixes in this, I had calculated the normals of the cylinders wrong and you now have more freedom on the prism to move the top of the triangle in any position between left (0.0) and right (1.0).

Currently my build of Godot 3 crashes on creating materials so I have no been able to test if there are any issues with the texturing and I have no updated my test project to Godot 3.0 yet as a result.

@BastiaanOlij
Copy link
Contributor Author

I will also be adding more objects in a later build, took me all morning to port this and I am running out of battery :)

One thing I do not like, and @reduz I hope you can make some suggestions here on how I can improve this, is that I rebuild the mesh on every property change. Is there a way to postpone this until the mesh is needed for the first time since the last property change?

@RameshRavone
Copy link
Contributor

STATIC_CHECKS: Clang-format is not happy 😄

@BastiaanOlij
Copy link
Contributor Author

Yeah Clang and I still need to learn to get along :) I did manage to get it installed the other day so I need to start playing with it and get it to go over my sourcecode before I commit it :)

@nunodonato
Copy link
Contributor

@BastiaanOlij check https://travis-ci.org/godotengine/godot/jobs/229333800 it tells you what you need to change to conform to clang style

@BastiaanOlij
Copy link
Contributor Author

@nunodonato I actually got clang-format working (I think) so my next commit should fingers crossed have all those issues fixed :)
I tried doing it by hand for one of my other PRs and its annoying:)

@BastiaanOlij
Copy link
Contributor Author

Ok, hopefully clang will love me now. I've also added a 3D plane which you can set segments for, together with a vertex shader that would make an ideal candidate for rendering height maps. I've also added a capsule object. Haven't been able to test if the textures are ok yet, recompiling on latest master atm, hoping it'll fix the crashing issue...


void register_primitives_types() {

ClassDB::register_class<Capsule3D>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, you don't want clang-format to like you, do you? :p

@akien-mga
Copy link
Member

Still got a couple things to fix to make clang-format happy: https://travis-ci.org/godotengine/godot/jobs/229399863
You should really look into that precommit hook to let it do formatting automatically :)

@BastiaanOlij
Copy link
Contributor Author

@akien-mga , how weird, I thought I had ran it on there, I must have made some last minute changes that caused something to misalign.

Yeah indeed, I need to figure out the precommit thingy, I was happy enough to get clang-format to get installed :)

@BastiaanOlij
Copy link
Contributor Author

Yeah! all green ticks:)

@BastiaanOlij
Copy link
Contributor Author

@reduz I had a quick look this morning about changing this over on the new structure as you suggested in #8578 , any suggestions of a class to look at that gives a bit of an idea of how the new approach works? Looks like you retired quadmesh? :)

I'll keep puzzling away in the mean while.

@reduz
Copy link
Member

reduz commented Jun 10, 2017 via email

@BastiaanOlij
Copy link
Contributor Author

BastiaanOlij commented Jun 10, 2017

@reduz only if its not too much trouble else I'm sure we'll catch eachother on IRC. Just need to get an idea of what the new structure is like.

edit, looks like the dirty way to implement it is to subclass ArrayMesh instead of Mesh. But I get a feeling the point is to implement the virtual methods on Mesh instead and just returning the single surface information that we generate from the properties set by the user? Then find some way to tell Godot the mesh has changed when the properties are changed which I'm guessing is done by these three methods:

	_clear_triangle_mesh();
	_change_notify();
	emit_changed();

(clear the previously generated mesh, set notify status and send signals?)

@BastiaanOlij
Copy link
Contributor Author

BastiaanOlij commented Jun 10, 2017

BROKEN -- Nearly all removed, will be rectified shortly

Just committed first attempt at changing implementation over to new approach. just working on cube3d atm which has been renamed to cubemesh.

It doesn't work yet, I'm not sure what is missing but I have a feeling _update isn't being called at the right time. @reduz if you wouldn't mind having a closer look, I must be missing a step somewhere but its getting late and my mind is giving up :)

Once I have cubemesh working I'll port all the others over.

@BastiaanOlij BastiaanOlij force-pushed the primitives_3.0 branch 3 times, most recently from bbdb1d1 to 147ca82 Compare June 11, 2017 13:32
@@ -0,0 +1,396 @@
/*************************************************************************/
/* cube3d.cpp */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yet another wrong filename in the header 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lolz, yeah after discussing with reduz we felt renaming this to cubemesh made more sense. The originals were all called something3d

But I'm far more interested in the golden tip that allows me to figure out what I'm doing wrong and why I'm not actually getting a cube created! :)

arr[VS::ARRAY_INDEX] = indices;

// out with the old
while (VisualServer::get_singleton()->mesh_get_surface_count(mesh) > 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any request of info from VisualServer (in this case mesh_get_surface_count()) stalls the rendering thread, so it's better avoided

@BastiaanOlij
Copy link
Contributor Author

Ok, we have a working cubemesh!! :)

I'll start working on converting the other primitives later in the week, should be pretty smooth sailing from here on. One of the suggestions made on IRC was to add a class in between Mesh and these primitives shapes as there will be some overlapping code. Not sure what a good name would be, suggestions are welcome :)

@Zireael07
Copy link
Contributor

PrimitiveMesh sound good as a class name?

@bojidar-bg
Copy link
Contributor

I guess GeneratedMesh is better?

@BastiaanOlij
Copy link
Contributor Author

The thing I'm most worried about regarding the name is that it has the potential to put people on the wrong foot. The thing that would make these classes easy to implement with the largest amount of common code in its base class would be to make the assumption any subclass will always have a single surface and single material. The subclass then simply implements the code that generates this surface.

GeneratedMesh suggests it would be able to do more. If I were to implement a dual contouring algorithm to generate the mesh for a terrain section (something on my wishlist of things to do) I might wish to create multiple surfaces.

The opposite goes for PrimitiveMesh, the base class has the potential to be used for more then just generating primitive shapes.

I'm probably overthinking it ;)

@BastiaanOlij BastiaanOlij force-pushed the primitives_3.0 branch 2 times, most recently from 46dce46 to a46285b Compare June 14, 2017 14:00
@BastiaanOlij
Copy link
Contributor Author

4 out of the original 6 mesh types have now been converted to the new approach.

@reduz asked to also add the original QuadMesh to this. PlaneMesh possibly replaces this as it can create a Quad if you subdivide each side by 1.

@bojidar-bg
Copy link
Contributor

Would the Quad and the TestCube nodes be obsoleted by the change?

@reduz
Copy link
Member

reduz commented Jun 14, 2017 via email

@BastiaanOlij
Copy link
Contributor Author

I think it's all done. Note that there is a test project to test this with here:

https://github.com/BastiaanOlij/TestPrimitives

@BastiaanOlij BastiaanOlij force-pushed the primitives_3.0 branch 2 times, most recently from 04fcbec to 8273cf3 Compare June 16, 2017 11:32
Adds the following resources:
- CapsuleMesh: a capsule object
- CubeMesh: a cube that can be subdivided
- CylinderMesh: a cylinder
- PlaneMesh: a horizontal plane that can be subdivided
- PrismMesh: a prism shape
- SphereMesh: a sphere
- QuadMesh: reintroduction of the original quadmesh

Removes the old Quad and TestCube nodes
@zatherz
Copy link
Contributor

zatherz commented Aug 17, 2017

@rraallvv
Copy link
Contributor

rraallvv commented Nov 12, 2017

Does anyone know of a demo video showing off this feature? Thanks.

@Zireael07
Copy link
Contributor

@rraallvv: you don't really need a demo video. Create a mesh instance node, click the arrow next to mesh: and voila, you can see all the primitives available.

@rraallvv
Copy link
Contributor

@Zireael07 I see... maybe it isn't available for 2.1, I'll try 3.0 then, thanks.

@Zireael07
Copy link
Contributor

This is only for 3.0, yes.

@aaronfranke
Copy link
Member

The Capsule primitive (and collider) are the wrong orientation by default. #5813

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants