Skip to content

Conversation

vis-prime
Copy link
Contributor

Description

RoundedBoxGeometry's parameters object by default holds BoxGeometry's parameters (since it extends BoxGeometry).
As a result:

  • the segments and radius values passed in the constructor are lost after geometry is created.
  • The toJSON method returns data relevant to creating a BoxGeometry.

This PR

  • Makes the parameters key hold the actual values passed to create the geometry.
  • Sets geometry.type to RoundedBoxGeometry instead of BoxGeometry.
  • The two changes above consequently fix the toJSON method and make it return the correct data.
  • The updated fromJSON method now returns the correct RoundedBoxGeometry instance instead of a BoxGeometry.

The geometry.parameters before and after for new RoundedBoxGeometry(1, 2, 3, 4, 0.125) :

para


Output from .toJSON():

compare


Output from RoundedBoxGeometry.fromJSON(data) Before and after:

from

@Mugen87 Mugen87 added this to the r179 milestone Jun 30, 2025
@vis-prime vis-prime marked this pull request as ready for review July 1, 2025 03:54
@vis-prime
Copy link
Contributor Author

vis-prime commented Jul 1, 2025

@Mugen87
It looks like #31319 introduces normal/shading artifacts.

Reverting this line:

super(width, height, depth, totalSegments, totalSegments, totalSegments)

Back to the original:

super(1, 1, 1, totalSegments, totalSegments, totalSegments)

fixes the issue .

Should we include this change in this PR or open a new one or something else?

2025-07-01.13-23-48.mp4

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 1, 2025

Should we include this change in this PR or open a new one or something else?

Let's include it! Good find, btw! 👍

@Mugen87 Mugen87 merged commit 6b2f433 into mrdoob:dev Jul 1, 2025
11 checks passed
@vis-prime vis-prime deleted the rounded-geometry-update-parameters branch July 2, 2025 05:00
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.

2 participants