Skip to content

feat(style): move style.point.model to its own category: 'model' - #2808

Open
ftoromanoff wants to merge 5 commits into
iTowns:masterfrom
ftoromanoff:feat/modelStyle
Open

feat(style): move style.point.model to its own category: 'model'#2808
ftoromanoff wants to merge 5 commits into
iTowns:masterfrom
ftoromanoff:feat/modelStyle

Conversation

@ftoromanoff

@ftoromanoff ftoromanoff commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Currently, to instanciate several object 3D at different position, the associated style is the style category 'point' with the parameter 'object'.
The use of extra parameter as 'orientation', 'size', 'scale' to cite a few is currently limited. That's the reason why I propose to add a new style category 'model' with it's own parameter. (As it's done for text and icon).

  • Shift the model.scale.set() to style.model.scale (to keep the origin size of the model)
  • add orientation of the model through 'up' and 'north' properties (as Vector3)

@ftoromanoff
ftoromanoff force-pushed the feat/modelStyle branch 2 times, most recently from af3c3a0 to c1ca507 Compare July 1, 2026 09:08
@ftoromanoff ftoromanoff changed the title feat(style): move style.point.model to its own category model feat(style): move style.point.model to its own category: 'model' Jul 1, 2026
Comment thread packages/Main/src/Converter/Feature2Mesh.js Outdated
Comment thread packages/Main/src/Converter/Feature2Mesh.js Outdated
Comment thread packages/Main/src/Converter/Feature2Mesh.js Outdated
Comment thread packages/Main/src/Converter/Feature2Mesh.js Outdated
Comment thread packages/Main/src/Converter/Feature2Mesh.js Outdated
@ftoromanoff
ftoromanoff force-pushed the feat/modelStyle branch 2 times, most recently from 96261e2 to fe81a26 Compare July 7, 2026 13:16
@ftoromanoff
ftoromanoff force-pushed the feat/modelStyle branch 5 times, most recently from ab49d98 to 1db5332 Compare July 28, 2026 12:12
@ftoromanoff
ftoromanoff marked this pull request as ready for review July 28, 2026 12:24
@ftoromanoff
ftoromanoff force-pushed the feat/modelStyle branch 10 times, most recently from ba7f2da to fc81a0b Compare July 30, 2026 09:58
@ftoromanoff
ftoromanoff requested a review from Neptilo July 31, 2026 10:40
Comment on lines +735 to +757
const styleModel = style.model;
const count = geometries.length;
const instancedMesh = new THREE.InstancedMesh(mesh.geometry, mesh.material, count);
let index = 0;
for (let i = 0; i < count * 3; i += 3) {
const mat = new THREE.Matrix4();
mat.setPosition(ptsIn[i], ptsIn[i + 1], ptsIn[i + 2]);
instancedMesh.setMatrixAt(index, mat);
index++;

for (let j = 0; j < count; j += 1) {
context.setGeometry(geometries[j]);
scale.set(1, 1, 1);
if (styleModel.size) {
scale.set(
styleModel.size.x / modelSize.x,
styleModel.size.y / modelSize.y,
styleModel.size.z / modelSize.z,
);
}

let headingRad = 0;
if (styleModel.heading) {
headingRad = styleModel.heading * THREE.MathUtils.DEG2RAD;
}
mat.makeRotationZ(-headingRad);
mat.setPosition(ptsIn[j * 3], ptsIn[j * 3 + 1], ptsIn[j * 3 + 2]);
mat.scale(scale.multiplyScalar(styleModel.scale));
instancedMesh.setMatrixAt(j, mat);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you use Vector3 methods to simplify your code

@ftoromanoff ftoromanoff Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done
scale.set(1, 1, 1) => scale.setScalar(1)
scale.set(x1/V.x, y1/V.y, z1/V.z) => scale.divide(V)

Comment thread examples/misc_instancing.html Outdated
Comment on lines +180 to +181
model: {
object: model,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could you rename var model to object to have model: { object,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

const modelObject = style.model.object;

// orientation of the model following up and north properties.
quaternion.setFromUnitVectors(style.model.up, zVect);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's possible to use Object3D.up ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried it to use with Object3D.lookAt(). But it has no direct impact to orient the object.

Comment on lines +782 to +787
const northWithRotation = style.model.north.applyQuaternion(quaternion.clone().conjugate());
const angletoNorth = northWithRotation.angleTo(yVect);
quaternionY.setFromAxisAngle(yVect, angletoNorth);
quaternion.multiply(quaternionY);

modelObject.setRotationFromQuaternion(quaternion);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This part seems complex; we could first orient the Z-axis locally toward the north, and then calculate the quaternion needed to orient the Z-axis in global space.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As we will need to calculate the size of the model object in the world space (using the bbox) we need the model oriented in the global space.

@ftoromanoff
ftoromanoff force-pushed the feat/modelStyle branch 5 times, most recently from 99029ea to ebe3491 Compare August 5, 2026 15:21
Comment thread packages/Main/src/Converter/Feature2Mesh.js Outdated
Comment thread packages/Main/src/Core/Style.js Outdated
Comment thread packages/Main/src/Converter/Feature2Mesh.js Outdated
Comment thread packages/Main/src/Core/Style.js Outdated
* @property {object|Function} model.heading - The heading (or azimuth) to orient the model in degree.
* @property {object|Function} model.scale - The value to scale the model. (default value is 1).
* @property {object|Function} model.up - The vector pointing up. (default value is Vector3(0, 0, 1)).
* @property {object|Function} model.north - The vector pointing north. (default value is Vector3(0, 1, 0)).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this north vector supposed to be tangent to the ground, so orthogonal to up? What happens if it's not?

@ftoromanoff ftoromanoff Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The user should be able to define a vector, that point toward the 'head' or the 'front' of the model. It's per defiition orthogonal to 'up'. Maybe we should better use 'head' or 'front' or an other name ?. 'north' is more the direction in the world referentiel where the head will be directed as for the meaning 'heading' which is the angle between the head and the north.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That makes sense. In that case, "front" sounds good. It matches the wording of most 3D modelling software. Then, the east vector in my code snippet in Feature2Mesh.js should be called right.
If the up and front vector are supposed to be orthogonal, I think we should either mention it in the doc, or check the input received by the function is valid.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can't imagine a real case where we would have the front of a model and a up, not being orthognal...

Ps: I renamed using front and right, it sounds better indeed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree that there are no such use cases, but it doesn't hurt to document the expected properties that the input values should verify. Or do a sanity check.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did add it in the documentation for 'front'.

Comment thread examples/misc_instancing.html Outdated
Comment thread packages/Main/src/Converter/Feature2Mesh.js Outdated
@ftoromanoff
ftoromanoff force-pushed the feat/modelStyle branch 4 times, most recently from d2832f3 to 94744e8 Compare August 27, 2026 09:32
@ftoromanoff

Copy link
Copy Markdown
Contributor Author

I just did a rebase to the main branch.
I remark that in Feature2Mesh.js, you removed almost all the context setup, (now done with refreshCollectionContext()), from the featureToExtrudedPolygon().

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants