-
-
Notifications
You must be signed in to change notification settings - Fork 36.1k
GridHelper: give different colors to X-centerLine and Z-centerLine #17075
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
Conversation
| colorGrid -- The color of the lines of the grid. This can be a [page:Color], a hexadecimal value and an CSS-Color name. Default is 0x888888 | ||
| colorXCenterLine -- The color of the X-centerline. This can be a [page:Color], a hexadecimal value and an CSS-Color name. Default is 0x444444 <br /> | ||
| colorGrid -- The color of the lines of the grid. This can be a [page:Color], a hexadecimal value and an CSS-Color name. Default is 0x888888 <br /> | ||
| colorZCenterLine -- The color of the Z-centerline. This can be a [page:Color], a hexadecimal value and an CSS-Color name. Default is 0x444444 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, the parameter order is now confusing. It should be at least: colorXCenterLine, colorZCenterLine, colorGrid. Or even better: colorGrid, colorXCenterLine, colorZCenterLine. However, this will break backwards compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, the parameter order is now confusing. It should be at least:
colorXCenterLine,colorZCenterLine,colorGrid. Or even better:colorGrid,colorXCenterLine,colorZCenterLine. However, this will break backwards compatibility.
I had noticed it too. Let me know if you need help with any code changes.
|
I think this would be a useful new feature. We can change the API in such a way the args can be checked and backwards-compatibility can be maintained. var options = { color: 0xffffff, colorXAxis: 0xff0000, colorZAxis: 0x0000ff );
new THREE.GridHelper( size, divisions, options );Or, we can just add a method to reset the z-axis color: gridHelper.setZAxisColor( color );Or, since it is just a helper, we can change order of the arguments and update the migration guide: new THREE.GridHelper( size, divisions, color, colorXAxis, colorZAxis );My preference is the last option. @mrdoob will have to make this decision. |
Then we have to adjust many examples (approx. 25) since instances of |
That will take 10 minutes.
Who knows, but it is not going to kill them. I think for this Helper we can change the order of the arguments for clarity and support the suggested feature. |
|
/ping @mrdoob Can you please state your preference? Do you want the feature? Which API? |
|
Considering #21431 (comment), this PR can be closed. |
With this modification I added a third parameter to the GridHelper construction, giving the possibility to set different colors to X-centerLine and Z-centerLine, like in Blender 2.80. This could be useful to mark the X-centerLine with the color of x-axis and the Z-centerLine with the color of z-axis.