scenerygraphics / scenery

Flexible VR Visualisation for Volumetric and Geometric Data on the Java VM, powered by Kotlin and Vulkan.
https://scenery.graphics
GNU Lesser General Public License v3.0
131 stars 32 forks source link

Fixed the sample method. #759

Closed odinsbane closed 3 months ago

odinsbane commented 5 months ago

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.

skalarproduktraum commented 5 months ago

Looks good to me, thank you, @odinsbane! I think it'd only need some formatting cleanup, and the -10 removed, right?

odinsbane commented 5 months ago

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 https://github.com/scenerygraphics/scenery/pull/759#discussion_r1636627451 :

@@ -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 https://github.com/scenerygraphics/scenery/pull/759#discussion_r1636628959 :

  • 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 https://github.com/scenerygraphics/scenery/pull/759#discussion_r1636629189 :

  • 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 https://github.com/scenerygraphics/scenery/pull/759#discussion_r1636629740 :

  • 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 https://github.com/scenerygraphics/scenery/pull/759#discussion_r1636630014 :

  • 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 https://github.com/scenerygraphics/scenery/pull/759#pullrequestreview-2113294272, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2NNEOZW7IX5UKH3SHDMLLZHBOHVAVCNFSM6AAAAABIQY27MSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCMJTGI4TIMRXGI . You are receiving this because you were mentioned.Message ID: @.***>

skalarproduktraum commented 3 months ago

Thanks @odinsbane! I took the liberty of fixing the remaining issues myself, I hope you don't mind.