Skip to content

Conversation

@mickae1
Copy link
Contributor

@mickae1 mickae1 commented Jan 14, 2025

fix bug when sinAngle is < 1. the value of expandWidth was too much:
limit it to the default: expandWidth = width * 0.5;

fix issue: #12427
and maybe this one : #7645

limit it to the default:  expandWidth = width * 0.5;
@github-actions
Copy link

Thank you for the pull request, @mickae1!

✅ We can confirm we have a CLA on file for you.

@ggetz
Copy link
Contributor

ggetz commented Jan 14, 2025

Thanks for the fix @mickae1! This fix looks reasonable to me. I'll test this out shortly to confirm.

@mickae1
Copy link
Contributor Author

mickae1 commented Jan 17, 2025

@ggetz

Thanks for the fix @mickae1! This fix looks reasonable to me. I'll test this out shortly to confirm.

a quick sandcastle to test it :

https://sandcastle.cesium.com/#c=7Vh7j9s2Ev8qhP+y0S2XoiiR8m6DS7dN2kOTFLtBUeB8aLUybQuVJUOPTZxiv/sNXzJFu4t8gGOQxCLnN2+OZvSUt+iplJ9ki75DtfyE7mRXDnv8m96br2aFfr5r6j4va9muZoubVb2qnwDXFbKWADN4rB/14fU1ejPURV82Neob1OVPEvU7iYp8L9scdX3ey1W9cSTq/E4fPaiT+QL9vaoRKpq66y1GH4AofYDQoelKBV062YYKu31cVE0NjK4M+bpsZXGJfjwIAMMhpBwOAUlbbnd9SKU3J4TPN86UppK4arbgUmMrMjY9gPHr5Wp2hf798OE97vq2rLfl5jj3DF8sNJdW9kNb+x6B7Wfr8LtWKnaHpjpWECi0lc1e9u3RhGrcngT5V7v71tLOtXudE7ulo7vL2x5+5XWMN22z/0FuWym7122bH3+SyuRuvqr/k+KUcpadVqI8ECfY/kgTzFP1CwijmCTecoRM/4oEFVgYykSwKD2tkTJ2PDm2PIEN5ac1UlJvWzPlcZRgakE0jcW4MoeJQkwWMWYQLOMJOa1RDAkhUcQJw7EVxESUsdMaUXEajUujqCCRM4lNMA4kMj66yXopjtyZ8OwxfqdJSmJnbyScwoKfGU6zSGBrJqHCXw6UiPi0NCkjNI6NwspxFxVmnJ6WBrGUUsypQYnER40KRl40tQKMJ8wmBqRSdkkUBwecHGq0IoIRnDILY2fKxOGupmRihBBBznOVn+VIwkkicGJdwSfp6ByYpqF0CE5CsBPFff/GDsRCp6c04hmmYxZG1pFZfCFoCQ29m2YsY2Oko/RCZsY2yXgqSObyMeKRvywpjUWQxHBnKXM3OBKXrhhYH3hHMJGk432JE+o8TZgIzFe3kbgr4l96e+sZzkyee4TpJMmYI+TUT9k0AIk486uPxTA/H0LIRMooJvYDkwU+4hCD89vGlGM9EHO1ixpMlJxXFIajKBTkQ4RfuohTjogQ4yUIx9TZTWgUB3SX8pBjlpzfzRjKQAj3agfHSexXem5B/KUyBbU/8u8TtaA0Cw3yiiCAEv8+pw6UnnmbxoRRLGyQgIBcgBG/4rJJYeZQJchZFYjBP2evEPeCAUhM+Tkk5n6FCl5+CsQvgZKzq60cYS8ZoCwjRRm6OSIZhF78V7UXCJlm5lO57ndLBK9PnezoSUJf8PlN0+7zfhk2Fe+gHWnLvHp9OMi8zetC4t9+vP/44+9/vPlw/+71R+DwvLiZdi/52LagEpo/BTL9i9v+2e5O+5i3wanpYxxm+U/tDi601LH7cU3SYqra0PXNHnW7fA1N8t5aZdQq9JmzdKqU2zXKbPLHtiyWroHtjwe5RKuZU+lBc4cu0Bx3zdAWQPCneYQG8sv+DydaP2xlPwrwD3+uD0OPJk8Lx+Rv9yPgtz/pb1n/IDf5UJ0kTPndmJwwCzz0XiVAVX6R67F3RDn0wFvd9CvzTuSbqsl71IGoCU/chUx/Ud1q0VRNe9p+kgXT/O7Uvpo8YGMeQTYiMv4Dj4sbxeEe9NEM0Ab+nlSZiPm1Kesekk2f78v1upKo2YzUaK5UJThZBFocFG6ihhMeqvEWeuU6UMQK0kxC3+jN+3xdDkY0oZrNA/jXaWZxE0O+r2Rt7YXGvRq9h3LY9uWFTvWkvsu7v0Bmt2+aftf18gBGJehbXyVlXYK+mW51Zj4xy8UVhqvNZujUVd2Xn+dj2HC7fbzy/Oc9K/kXWeXVYZefMconbPKAyYmNnZsct1HAs/nxpxnULtYjSOSxfEH563dmjjUlYVoLPMKLQ9Z5PTSVwXFZBtVkos/r9dpG3s5xMFarZzOC5x1UzkNb7uHuPUGG6108bnQ4X6/nSpavljud27IQlljIonDLVqeTpUvvtzvsjnWxa5u6GYDDJq86eWVuY63q51jfPbLqqAMAV8x+WzAz9Zem2X9s5vZJ1j2oKztDYv78/zvDhe8MUP27LZjwwucETbuWj8N2K9uv/UDxjeL7FR8i/Ii0EjK6fTEolsSPi6+qicTU+k726vOUS9w1cCjr3Pjdw45Rsp5s2hKSyNKN70IvZj523L5yhCpWPsVwsEfPJgALzwcPYyYObSvrPjBefzxTbvWsVjk3cxqvZkt4+gz/JUy1pVwN/QJGnQRau9kR9qEHhpYNpzSJYHjIOPTLsy+wDxNSIgjFEcsISwlNKKg3G605Mf6WqG8SUZZy6FHVjJxZznDAI9hlCeEkS0im5h7NGyo/8ITJPYZJF4alNFK8h8OJKUAZT6M4pdAqw8RFPKYkhplCcEKzmItE8HjkmqYxTBacZjyGWZhqrjrDp9omMAtwRoB5BKydIwCepYyKlEfgC56pqd1pm9BU8CQDt8UZTOPP6pI8wPuwyLsekh2q4semqR7z9vuh75saMl8HzkRFfZIbs9TVijBq6m0ZVpUbv3DfezdAg4NUeFkdh35Bo3IDPUqg1cKl9wvXGt7668sfHs+4uVfyhat6RqtJn5GEsn9ZiffNxAPGKdh8VtavZOu7rxGGbmZXs9uuP1bylbum/yr3h6bt0dBWc4yve7k/VEDbXT8OxV8SimY3diu31z70dl0+oXL93YVP3qio8q6Dk81QVaoRW81e3V4D/RlUeRXc+AEmoyo/KrJd9OoXs4kxvr2Gx8vI3oQ+4Pw/

@ggetz
Copy link
Contributor

ggetz commented Jan 21, 2025

Thanks @mickae1! I was able to verify this is working in your example, as well as with existing polyline dash materials and the meridian edge case.

Please add a summary to CHANGES.md and yourself to CONTRIBUTORS.md if you haven't done so already. Then this should be all good to merge!

@ggetz ggetz self-assigned this Jan 21, 2025
@mickae1
Copy link
Contributor Author

mickae1 commented Jan 22, 2025 via email

@ggetz
Copy link
Contributor

ggetz commented Jan 23, 2025

@mickae1 Sure. You can link to the fixed issue as well. Thanks!

- Fixed error in polyline when sinAngle is < 1. the value of expandWidth was too much. [#12434](#12434)
@mickae1
Copy link
Contributor Author

mickae1 commented Jan 25, 2025

@ggetz it's done :)

@ggetz
Copy link
Contributor

ggetz commented Jan 27, 2025

Great! Thank you @mickae1!

@ggetz ggetz added this pull request to the merge queue Jan 27, 2025
Merged via the queue into CesiumGS:main with commit e68a5ab Jan 27, 2025
4 checks passed
@mickae1 mickae1 deleted the polyline branch January 27, 2025 17:23
@GISALLNERD
Copy link

Hi @ggetz, @mickae1

After this commit and the release of version 1.126, we are experiencing problems with "PolylineGlowMaterialProperty" - Glow Polyline property of the Polyline please check the below picks, when the polyline is with two points & straight line there is no problem, but if their is multiple vertex and angles then then the polyline is deformed and irregular widths

It is more prominent at different camera angles, when zoomed out and when the line width is increased.

---- Cesium 1.126
GlowLineNew40

---- Cesium 1.126
Glowline2_1_126

---- Cesium 1.125
GlowPolyline2_1_125

---- Cesium 1.126
GlowLineNew401

---- Cesium 1.125
GlowPolyline3_1_125

----SandCastle
https://sandcastle.cesium.com/#c=lVNdb9owFP0rVp5SiRnbxPkaReuYtIdRDWlTX5o9mMSANcdGtgOiFf99DgkFtVTqFCn2vfec4+vk3FIr68BW8B034BYovgNTbkVTw4djLiyC8hhPtXJMKG6K4OZzoc5PeVRYSb0TajXzCC/T6UGunHCCW8iqKnwuFACK1TwHRfC9g4OFbDiQLUkr4NYc2MYsWcmLYNDCN1ru22oOjuw2Yb2iPzE/tTllxvkdUyO4NLr+xleGc3tnDNuH4LFjAXBaAfiUIkiSDEcEZyMU0WxwrpEYEkxpgrAv0CSL0lNt8L4QQWSUUEziK0pxRhIaoxGiHxHClKA4QzR5KxThDKcpRTh6X+jlZqMoiQlOKb2iQ5IoSRGm2Qca+r9P1K1/bnrwTlRunYMI9XHNHDeCyfzSY/P+/7Z2uO8Bc6M33Lh9+HxSbr01195QOUDwfCnHPK7P45dsqaU2Z3e0Efw6u5v+6BGHrsGDfx8ujdxb9knr+rcOXxm4BQaDYGzdXvLJ6agvot5o40BjZAjh0PF6I/0l7HDRlH+5g6U9ElvoeHhJHVdiC0R1e2W2QCmZtb6ybKT8JZ78JEzGQ49/Q5WaVX6Efm65kWzfwtZ4MuuSEMLx0IfXmU5ruWDmlfI/

Can we revert back with the changes for the upcoming release and till @mickae1 fixes the thing.

We won't be able to update to the latest version of the Cesium version because of this, as we are using this to hi-light
the building footprint

@ggetz
Copy link
Contributor

ggetz commented Mar 3, 2025

Thanks for brining this up @GISALLNERD. I'm going to open a new item to track this.

I'm hesitant to revert this immediately as we are in the process of this month's release.

@ggetz
Copy link
Contributor

ggetz commented Mar 3, 2025

Opened #12506 to track.

@GISALLNERD
Copy link

Hi @ggetz, can we please revert this update now for the up coming release as there is no update from @mickae1 on this issue.

@mickae1
Copy link
Contributor Author

mickae1 commented Apr 26, 2025

Hi @ggetz, can we please revert this update now for the up coming release as there is no update from @mickae1 on this issue.

Yes you can revert, it's a complicate modification, and I'm not free to do it now. And based of all the post before, the revert will not fix everything

@ggetz
Copy link
Contributor

ggetz commented Apr 30, 2025

Thanks for the update @mickae1. I'll take a look at reverting this for tomorrow's release.

github-merge-queue bot pushed a commit that referenced this pull request Apr 30, 2025
Revert #12434, fix issue with polyline material width
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.

3 participants