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.

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

Fix wrong y-axis range in writeImage() on Windows #39

Merged
merged 5 commits into from
Mar 5, 2023

Conversation

nocd5
Copy link
Contributor

@nocd5 nocd5 commented Feb 6, 2023

for y := 0; y < height; y++ { height-y }
returns height ~ 1

for y := 0; y < height; y++ { height-y }
returns height ~ 1
clipboard_windows.go Outdated Show resolved Hide resolved
@changkun
Copy link
Member

changkun commented Feb 6, 2023

It looks like some failure tests are real.

@nocd5
Copy link
Contributor Author

nocd5 commented Feb 7, 2023

I think a following code fix the problem.
But the test still be failed though (R,G,B,A) is matched.

diff --git a/clipboard_windows.go b/clipboard_windows.go
index b7a8d5e..70fbd18 100644
--- a/clipboard_windows.go
+++ b/clipboard_windows.go
@@ -145,7 +145,7 @@ func readImage() ([]byte, error) {
 			// xhat := (x + int(info.Width-3)) % int(info.Width)
 
 			xhat := (x + int(info.Width)) % int(info.Width)
-			yhat := int(info.Height) - y
+			yhat := int(info.Height) - 1 - y
 			r := data[idx+2]
 			g := data[idx+1]
 			b := data[idx+0]
--- FAIL: TestClipboard (0.75s)
    --- FAIL: TestClipboard/image (0.66s)
        clipboard_test.go:102: read data from clipbaord is inconsistent with previous written data, pix: (0,0), got: {R:30 G:30 B:30 A:255}, want: {R:30 G:30 B:30 A:255}
        clipboard_test.go:102: read data from clipbaord is inconsistent with previous written data, pix: (0,1), got: {R:30 G:30 B:30 A:255}, want: {R:30 G:30 B:30 A:255}
        clipboard_test.go:102: read data from clipbaord is inconsistent with previous written data, pix: (0,2), got: {R:30 G:30 B:30 A:255}, want: {R:30 G:30 B:30 A:255}
        clipboard_test.go:102: read data from clipbaord is inconsistent with previous written data, pix: (0,3), got: {R:30 G:30 B:30 A:255}, want: {R:30 G:30 B:30 A:255}
...
        clipboard_test.go:102: read data from clipbaord is inconsistent with previous written data, pix: (957,196), got: {R:30 G:30 B:30 A:255}, want: {R:30 G:30 B:30 A:255}
        clipboard_test.go:102: read data from clipbaord is inconsistent with previous written data, pix: (957,197), got: {R:30 G:30 B:30 A:255}, want: {R:30 G:30 B:30 A:255}
        clipboard_test.go:111: read data from clipboard contains too much inconsistent pixels to the previous written data, number of incorrect pixels: 189684

@nocd5
Copy link
Contributor Author

nocd5 commented Feb 7, 2023

I think a following code fix the problem. But the test still be failed though (R,G,B,A) is matched.

diff --git a/clipboard_windows.go b/clipboard_windows.go
index b7a8d5e..70fbd18 100644
--- a/clipboard_windows.go
+++ b/clipboard_windows.go
@@ -145,7 +145,7 @@ func readImage() ([]byte, error) {
 			// xhat := (x + int(info.Width-3)) % int(info.Width)
 
 			xhat := (x + int(info.Width)) % int(info.Width)
-			yhat := int(info.Height) - y
+			yhat := int(info.Height) - 1 - y
 			r := data[idx+2]
 			g := data[idx+1]
 			b := data[idx+0]
--- FAIL: TestClipboard (0.75s)
    --- FAIL: TestClipboard/image (0.66s)
        clipboard_test.go:102: read data from clipbaord is inconsistent with previous written data, pix: (0,0), got: {R:30 G:30 B:30 A:255}, want: {R:30 G:30 B:30 A:255}
        clipboard_test.go:102: read data from clipbaord is inconsistent with previous written data, pix: (0,1), got: {R:30 G:30 B:30 A:255}, want: {R:30 G:30 B:30 A:255}
        clipboard_test.go:102: read data from clipbaord is inconsistent with previous written data, pix: (0,2), got: {R:30 G:30 B:30 A:255}, want: {R:30 G:30 B:30 A:255}
        clipboard_test.go:102: read data from clipbaord is inconsistent with previous written data, pix: (0,3), got: {R:30 G:30 B:30 A:255}, want: {R:30 G:30 B:30 A:255}
...
        clipboard_test.go:102: read data from clipbaord is inconsistent with previous written data, pix: (957,196), got: {R:30 G:30 B:30 A:255}, want: {R:30 G:30 B:30 A:255}
        clipboard_test.go:102: read data from clipbaord is inconsistent with previous written data, pix: (957,197), got: {R:30 G:30 B:30 A:255}, want: {R:30 G:30 B:30 A:255}
        clipboard_test.go:111: read data from clipboard contains too much inconsistent pixels to the previous written data, number of incorrect pixels: 189684

Type is different.
Comapre with go-cmp

--- FAIL: TestClipboard (1.82s)
    --- FAIL: TestClipboard/image (1.71s)
        clipboard_test.go:103: User value is mismatch (-want +got):
              any(
            - 	color.NRGBA{R: 30, G: 30, B: 30, A: 255},
            + 	color.RGBA{R: 30, G: 30, B: 30, A: 255},
              )
        clipboard_test.go:103: User value is mismatch (-want +got):
              any(
            - 	color.NRGBA{R: 30, G: 30, B: 30, A: 255},
            + 	color.RGBA{R: 30, G: 30, B: 30, A: 255},
              )

`for y := 0; y < int(info.Height); y++ { yhat := int(info.Height)  - y }`
returns height ~ 1
Absorb inconsistent color type
@nocd5
Copy link
Contributor Author

nocd5 commented Feb 7, 2023

readImage() and clipboard_test.go are updated.
If you have better solution for the problem, please reject the PR.

@changkun
Copy link
Member

changkun commented Feb 7, 2023

What if we just update the test file?

@nocd5
Copy link
Contributor Author

nocd5 commented Feb 7, 2023

What if we just update the test file?

A type of img2.At() as got is color.RGBA and type of img1.At() as want is color.NRGBA.

So I tried to convert to RGBA64 and compare.
But RGBA64At() is not declared at go 1.16.
I'll compare R,G,B,A each value.

It seems img2.At() returns type vary depending the image has transparent color or not.
If it has transparent, returns color.NRGBA, else returns color.RGBA.
A png image for test has no transparent but img2.At() for out of range returns {R:0,G:0,B:0,A:0}.
So before my patch img2.At() returns color.NRGBA and test is passed.

nocd5 and others added 2 commits February 7, 2023 17:49
@changkun changkun merged commit 72ffba4 into golang-design:main Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants