-
Notifications
You must be signed in to change notification settings - Fork 32
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
Fixed the sample method. #759
Fixed the sample method. #759
Conversation
with odd h values. Issue scenerygraphics#750 Made a hot fix for generating an image form the ColormapEditor the image was only halfdrawn. Issue scenerygraphics#757
will slice from the current position. Which might not be zero.
Looks good to me, thank you, @odinsbane! I think it'd only need some formatting cleanup, and the -10 removed, right? |
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.
Just some minor comments. Thanks @odinsbane 👍
Great! I don't have a kotlin development environment so I need to pay
attention to the formatting a little.
The "-10" is in the draw background and it is at the point of creating the
buffered image. It should probably be fixed so the background image is
filled and the buffered image is the correct size. Without the -10 the
markers get painted over. It might take a little refactoring to remove.
…On Wed, Jun 12, 2024, 4:53 PM Ulrik Günther ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Just some minor comments. Thanks @odinsbane <https://github.com/odinsbane>
👍
------------------------------
In src/main/kotlin/graphics/scenery/volumes/ColormapPanel.kt
<#759 (comment)>
:
> @@ -304,8 +304,8 @@ class ColormapPanel(val target:Volume?): JPanel() {
internal fun toImage(): BufferedImage {
val rec: Rectangle = this.bounds
- val bufferedImage = BufferedImage(rec.width, rec.height, BufferedImage.TYPE_INT_ARGB)
- paintBackgroundGradient(colorPoints.sortedBy { it.position }, bufferedImage.graphics as Graphics2D)
+ val bufferedImage = BufferedImage(rec.width, rec.height - 10, BufferedImage.TYPE_INT_ARGB)
When the sampling is fixed, the -10 can go away again I think
------------------------------
In src/main/kotlin/graphics/scenery/volumes/Colormap.kt
<#759 (comment)>
:
>
- val c1 = Vector4f(ub[0].toFloat() / 255.0f, ub[1].toFloat() / 255.0f, ub[2].toFloat() / 255.0f, ub[3].toFloat() / 255.0f)
- val c2 = Vector4f(ub[4].toFloat() / 255.0f, ub[5].toFloat() / 255.0f, ub[6].toFloat() / 255.0f, ub[7].toFloat() / 255.0f)
+ val b = buffer.duplicate()
+ b.position(globalOffset + previous * 4);
+ val color = ByteArray(4)
+ b.get(color);
+ //Add to "Image" utility class?
+ val c1 = Vector4f( color[0].toUByte().toFloat() / 255f, color[1].toUByte().toFloat() / 255f, color[2].toUByte().toFloat() / 255f, color[3].toUByte().toFloat() / 255f)
+
+ if( bufferPosition > previous ){
Formatting should be adjusted to the general format of the project (sorry
we don't have a style file :-/)
------------------------------
In src/main/kotlin/graphics/scenery/volumes/Colormap.kt
<#759 (comment)>
:
>
- val c1 = Vector4f(ub[0].toFloat() / 255.0f, ub[1].toFloat() / 255.0f, ub[2].toFloat() / 255.0f, ub[3].toFloat() / 255.0f)
- val c2 = Vector4f(ub[4].toFloat() / 255.0f, ub[5].toFloat() / 255.0f, ub[6].toFloat() / 255.0f, ub[7].toFloat() / 255.0f)
+ val b = buffer.duplicate()
+ b.position(globalOffset + previous * 4);
+ val color = ByteArray(4)
+ b.get(color);
+ //Add to "Image" utility class?
+ val c1 = Vector4f( color[0].toUByte().toFloat() / 255f, color[1].toUByte().toFloat() / 255f, color[2].toUByte().toFloat() / 255f, color[3].toUByte().toFloat() / 255f)
+
+ if( bufferPosition > previous ){
+ //interpolate fraction part.
+ b.get(color);
Semicolon is not necessary
------------------------------
In src/main/kotlin/graphics/scenery/volumes/Colormap.kt
<#759 (comment)>
:
>
- val c1 = Vector4f(ub[0].toFloat() / 255.0f, ub[1].toFloat() / 255.0f, ub[2].toFloat() / 255.0f, ub[3].toFloat() / 255.0f)
- val c2 = Vector4f(ub[4].toFloat() / 255.0f, ub[5].toFloat() / 255.0f, ub[6].toFloat() / 255.0f, ub[7].toFloat() / 255.0f)
+ val b = buffer.duplicate()
+ b.position(globalOffset + previous * 4);
+ val color = ByteArray(4)
+ b.get(color);
+ //Add to "Image" utility class?
+ val c1 = Vector4f( color[0].toUByte().toFloat() / 255f, color[1].toUByte().toFloat() / 255f, color[2].toUByte().toFloat() / 255f, color[3].toUByte().toFloat() / 255f)
+
+ if( bufferPosition > previous ){
+ //interpolate fraction part.
+ b.get(color);
+ val c2 = Vector4f( color[0].toUByte().toFloat() / 255f, color[1].toUByte().toFloat() / 255f, color[2].toUByte().toFloat() / 255f, color[3].toUByte().toFloat() / 255f)
Should not have space after the brackets. Sorry again for not having a
style file.
------------------------------
In src/main/kotlin/graphics/scenery/volumes/Colormap.kt
<#759 (comment)>
:
>
- val c1 = Vector4f(ub[0].toFloat() / 255.0f, ub[1].toFloat() / 255.0f, ub[2].toFloat() / 255.0f, ub[3].toFloat() / 255.0f)
- val c2 = Vector4f(ub[4].toFloat() / 255.0f, ub[5].toFloat() / 255.0f, ub[6].toFloat() / 255.0f, ub[7].toFloat() / 255.0f)
+ val b = buffer.duplicate()
+ b.position(globalOffset + previous * 4);
+ val color = ByteArray(4)
+ b.get(color);
+ //Add to "Image" utility class?
+ val c1 = Vector4f( color[0].toUByte().toFloat() / 255f, color[1].toUByte().toFloat() / 255f, color[2].toUByte().toFloat() / 255f, color[3].toUByte().toFloat() / 255f)
+
+ if( bufferPosition > previous ){
+ //interpolate fraction part.
+ b.get(color);
+ val c2 = Vector4f( color[0].toUByte().toFloat() / 255f, color[1].toUByte().toFloat() / 255f, color[2].toUByte().toFloat() / 255f, color[3].toUByte().toFloat() / 255f)
+ return c1.lerp(c2, bufferPosition - previous.toFloat())
+ } else{
Space missing before {.
—
Reply to this email directly, view it on GitHub
<#759 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA2NNEOZW7IX5UKH3SHDMLLZHBOHVAVCNFSM6AAAAABIQY27MSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMJTGI4TIMRXGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
markers. This creates a variable 'markerSpace' to show where the magic -10 appears.
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.
lgtm execpt those two comments. I like that magic numbers get replaced with propper variable names :)
9abda09
to
5fc60bf
Compare
Thanks @odinsbane! I took the liberty of fixing the remaining issues myself, I hope you don't mind. |
Colormap.sample would go out of bounds. It was also broken for odd h values. Issue #750
Made a hot fix for generating an image form the ColormapEditor the image was missing 10px so depending on how large the editor was displayed the resulting Colormap always returned black. Issue #757
If this looks ok I can improve it.