Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix putSliceWithBorders extention function for Bitmap class #2144

Merged
merged 2 commits into from
Feb 13, 2024
Merged

Fix putSliceWithBorders extention function for Bitmap class #2144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
65ca3e1
Fix putSliceWithBorders
jobe-m Feb 6, 2024
1fc6700
Replace Bitmap1 with Bitmap8 in unit test
jobe-m Feb 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions korge-core/src/korlibs/image/bitmap/BitmapExt.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ fun Bitmap.putSliceWithBorder(x: Int, y: Int, bmp: BmpSlice, border: Int = 1) {
// Vertical replicate
for (n in 1..border) {
val rwidth = width + border * 2
this.copy(x, y, this, x, y - n, rwidth, 1)
this.copy(x, y + height - 1, this, x, y + height - 1 + n, rwidth, 1)
this.copy(x - border, y, this, x - border, y - n, rwidth, 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't be rwidth + border * 2?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you exatly mean should I put that term?
I did the fix in a rush. Let me check it carefully with a bit more time on Friday :)
I will create also a "better" (more colorful) tile for visually checking if the expansion of the border works as we expect.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

fun Bitmap.copy(srcX: Int, srcY: Int, dst: Bitmap, dstX: Int, dstY: Int, width: Int, height: Int)

I mean, if border was not taken into account and if position is displaced now, width might need to be extended?

For example, let's imagine a segment: x=10, width=10 (right = 20)
If I want to add one pixel on both sides to the line: x=9, width=12 (right=21)

So subtracting N to the x (left), requires adding N*2 to the width, or N to the right coordinate.
In the case we are extending the same amount of pixels on both sides of the segment.

Not sure if that was the case here, but asked just in case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. I checked now after my Friday stream again and my implementation actually adds the border twice. So, if I add border = 2 than 4 pixels are added to each side of the tile :)

I need to check again...

Copy link
Member Author

@jobe-m jobe-m Feb 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the border error from last Friday anymore.

I checked again now and I could not find a problem anymore with my initial changes. Here is a screenshot for border = 1:

image

So it expands correctly by one pixel in each direction. I checked that visually with border = 2, 3 and 5 and it was correctly expanding with 2 resp. 3 and 5 pixels.

this.copy(x - border, y + height - 1, this, x - border, y + height - 1 + n, rwidth, 1)
}
}

Expand Down
28 changes: 28 additions & 0 deletions korge-core/test/korlibs/image/bitmap/BitmapSliceTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import korlibs.math.geom.vector.*
import korlibs.platform.*
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertTrue

class BitmapSliceTest {
val logger = Logger("BitmapSliceTest")
Expand All @@ -27,6 +28,33 @@ class BitmapSliceTest {
assertEquals("Rectangle(x=24, y=24, width=24, height=24)", bmp.sliceWithSize(16, 16, 32, 32).sliceWithSize(8, 8, 40, 40).bounds.toString())
}

@Test
fun testPutSliceWithBorder() {
Copy link
Member

@soywiz soywiz Feb 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding test. Maybe the Bitmap32 can be converted into a Bitmap1/Bitmap8 and then:

korge/korge-core/test/korlibs/image/bitmap/Bitmap1Test.kt

Lines 27 to 34 in 11993ae

assertEquals(
"""
X...
.X..
.X..
...X
""".trimIndent(),
bmp.toLines(".X").joinToString("\n")

korge/korge-core/src/korlibs/image/bitmap/Bitmap8.kt

Line 49 in 65ca3e1

fun Bitmap.tryToExactBitmap8(): Bitmap8? {

bmp.tryToExactBitmap8().toLines(".*#")!!.joinToString("\n")

or use a reference image. Since we are in korge-core, we should be able to generate screenshots.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked using Bitmap1 in the unit test but putWithBorder() is not implemented for Bitmap1. What seems to be missing is the implementation of slice() for Bitmap1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Slice() is not implemented for Bitmap1 I would like to keep the unit test as it is now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can try with Bitmap8 indeed and create an imput and output image with indexes...

// Create bitmap with border pixels
val bmpInput = Bitmap8(4, 4, byteArrayOf(
1, 2, 3, 4,
5, 0, 0, 6,
7, 0, 0, 8,
9, 1, 2, 3
))

val extruded = Bitmap8(6, 6)
extruded.putSliceWithBorder(1, 1, bmpInput.slice(), 1)

assertTrue {
extruded.contentEquals(
Bitmap8(6, 6, byteArrayOf(
1, 1, 2, 3, 4, 4,
1, 1, 2, 3, 4, 4,
5, 5, 0, 0, 6, 6,
7, 7, 0, 0, 8, 8,
9, 9, 1, 2, 3, 3,
9, 9, 1, 2, 3, 3
))
)
}
}

@Test
fun testBmpSize() {
val slice = Bitmap32(128, 64, premultiplied = false).sliceWithSize(24, 16, 31, 17)
Expand Down
Loading