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

internal/ui: a wrong calculation of a region for anti-aliasing #2679

Closed
1 of 10 tasks
divVerent opened this issue Jun 23, 2023 · 20 comments · Fixed by #2681
Closed
1 of 10 tasks

internal/ui: a wrong calculation of a region for anti-aliasing #2679

divVerent opened this issue Jun 23, 2023 · 20 comments · Fixed by #2681

Comments

@divVerent
Copy link
Contributor

Ebitengine Version

v2.5.4

Operating System

  • Windows
  • macOS
  • Linux
  • FreeBSD
  • OpenBSD
  • Android
  • iOS
  • Nintendo Switch
  • Xbox
  • Web Browsers

Go Version (go version)

go version go1.20.5 darwin/arm64

What steps will reproduce the problem?

Run AAAAXY in the iOS Simulator. Issue doesn't happen on real iPhone 6S or real iPad Air 1st Gen. Walk right and jump until you touch the first checkpoint, then click the back button at the top left and click "Play".

What is the expected result?

A map screen shows up.

What happens instead?

Game halts, and debugger shows:

-[MTLDebugRenderCommandEncoder setScissorRect:]:3814: failed assertion `Set Scissor Rect Validation
(rect.x(0) + rect.width(800))(800) must be <= render pass width(128)
(rect.y(0) + rect.height(384))(384) must be <= render pass height(64)

Anything else you feel useful to add?

Issue already exists in v2.5.0. I cannot go back further. Do not yet know what exact render call triggers it.

Size 800x384 surprises me - there's no object in my game of that size. Also don't know yet which render pass this is.

@divVerent
Copy link
Contributor Author

Last render frame dump:

Update count per frame: 2
Internal image sizes:
  3: (1024, 512)
  4: (2778, 1284)
  7: (8192, 1024)
  11: (2048, 512)
  12: (1024, 512)
  13: (128, 64)
  14: (128, 128)
  15: (128, 128)
  16: (256, 128)
  17: (512, 256)
  18: (512, 256)
  19: (512, 512)
  20: (512, 512)
  21: (1024, 512)
Graphics commands:
  draw-triangles: dst: 11 <- src: [7, (nil), (nil), (nil)], num of dst regions: 2, num of indices: 12, blend: {src-color: 1, src-alpha:  1, dst-color: 0, dst-alpha: 0, op-color: 0, op-alpha: 0}, even-odd: false
  draw-triangles: dst: 11 <- src: [7, (nil), (nil), (nil)], num of dst regions: 1, num of indices: 4488, blend: {src-color: 1, src-alpha:  1, dst-color: 5, dst-alpha: 5, op-color: 0, op-alpha: 0}, even-odd: false
  draw-triangles: dst: 11 <- src: [7, (nil), (nil), (nil)], num of dst regions: 1, num of indices: 60, blend: {src-color: 1, src-alpha:  1, dst-color: 5, dst-alpha: 5, op-color: 0, op-alpha: 0}, even-odd: false
  draw-triangles: dst: 11 <- src: [7, (nil), (nil), (nil)], num of dst regions: 1, num of indices: 54, blend: {src-color: 1, src-alpha:  1, dst-color: 5, dst-alpha: 5, op-color: 0, op-alpha: 0}, even-odd: false
  draw-triangles: dst: 12 <- src: [7, 11, 7, (nil)], num of dst regions: 1, num of indices: 6, blend: {src-color: 1, src-alpha:  1, dst-color: 0, dst-alpha: 0, op-color: 0, op-alpha: 0}, even-odd: false
  draw-triangles: dst: 11 <- src: [12, (nil), (nil), (nil)], num of dst regions: 1, num of indices: 6, blend: {src-color: 1, src-alpha:  1, dst-color: 0, dst-alpha: 0, op-color: 0, op-alpha: 0}, even-odd: false
  draw-triangles: dst: 12 <- src: [11, (nil), (nil), (nil)], num of dst regions: 1, num of indices: 6, blend: {src-color: 1, src-alpha:  1, dst-color: 0, dst-alpha: 0, op-color: 0, op-alpha: 0}, even-odd: false
  draw-triangles: dst: 12 <- src: [7, (nil), (nil), (nil)], num of dst regions: 1, num of indices: 342, blend: {src-color: 1, src-alpha:  1, dst-color: 5, dst-alpha: 5, op-color: 0, op-alpha: 0}, even-odd: false
  draw-triangles: dst: 12 <- src: [11, (nil), (nil), (nil)], num of dst regions: 1, num of indices: 6, blend: {src-color: 1, src-alpha:  1, dst-color: 5, dst-alpha: 5, op-color: 0, op-alpha: 0}, even-odd: false
  new-image: result: 13, width: 96, height: 64, screen: false
  draw-triangles: dst: 13 <- src: [(nil), (nil), (nil), (nil)], num of dst regions: 1, num of indices: 6, blend: {src-color: 0, src-alpha:  0, dst-color: 0, dst-alpha: 0, op-color: 0, op-alpha: 0}, even-odd: false

@divVerent
Copy link
Contributor Author

The new-image is likely whiteImage here: https://github.com/divVerent/aaaaxy/blob/main/internal/menu/map.go#L101

Commenting out all that has to do with it works around the bug.

Commenting out the contents of DrawPolyLine works around the bug: https://github.com/divVerent/aaaaxy/blob/main/internal/engine/polyline.go#L28

@hajimehoshi
Copy link
Owner

Hmm, interesting.

  • Is this bug reproducible with other OSes like macOS?
  • Would it be possible to have a minimized test case for this?

@hajimehoshi hajimehoshi added this to the v2.5.5 milestone Jun 23, 2023
@divVerent
Copy link
Contributor Author

divVerent commented Jun 23, 2023

Not reproducible on macOS, but likely this is an optional debug feature (MTLDebugRenderCommandEncoder) I do not know how to trigger on macOS.

Minimized test case is unlikely, as I do not yet know why it happens. The DrawPolyLine call to DrawTriangles looks entirely harmless to me. Quite possible it's internal state messup inside Ebitengine.

@divVerent
Copy link
Contributor Author

Hm...

(rect.x(0) + rect.width(800))(800) must be <= render pass width(128)
(rect.y(0) + rect.height(384))(384) must be <= render pass height(64)

Any chance I can get this "render pass width" from Go code? I guess it'd be much easier if we could just panic in Go code there, to get a proper Go backtrace.

@hajimehoshi
Copy link
Owner

Any chance I can get this "render pass width" from Go code?

I have no idea about this.

We might be able to investigate this by using MTLDebugRenderCommandEncoder explicitly, but I'm not familiar with this...

@divVerent
Copy link
Contributor Author

divVerent commented Jun 23, 2023

800x384 makes no sense at all here BTW - nothing in my game has this size. Typical screen texture is 640x360. And after changing to the iPad emulator, even in portrait mode, it's still 800x384 in the error.

128x64 feels like some sort of minimum texture size used for whiteImage.

@divVerent
Copy link
Contributor Author

Issue still happens (including new-image call) if I move whiteImage elsewhere and initialize it at game start. That's weird.

@divVerent
Copy link
Contributor Author

divVerent commented Jun 23, 2023

One recent change to that code: I had toggled AntiAlias: false to AntiAlias: true in the DrawTriangles options to get antialiased lines.

Reverting that stops the bug from happening.

Too bad I don't know how to run the examples in the iOS Simulator without starting new Xcode project and doing lots of work there.

divVerent added a commit to divVerent/aaaaxy that referenced this issue Jun 23, 2023
It crashes at least in the Simulator.

Works around hajimehoshi/ebiten#2679
@hajimehoshi
Copy link
Owner

hajimehoshi commented Jun 23, 2023

Using anti-aliasing means using a double-sized offscreen image.

@divVerent
Copy link
Contributor Author

Yeah, even then, 400x192 also makes no sense.

@divVerent
Copy link
Contributor Author

Grounding this call:

https://github.com/hajimehoshi/ebiten/blob/main/internal/ui/image.go#L400

Prevents the crash.

@divVerent
Copy link
Contributor Author

divVerent commented Jun 23, 2023

Randomized stuff so only one draw call takes place per frame, to rule out interference of draw calls with each other.

Caught one of those with these variable contents:

Update count per frame: 1
DrawTriangles:
- i.region: {X:208 Y:48 Width:272 Height:224}
- i.image bounds: 544x448
- src[0] bounds: 1x1
- vertices: [127.17255 298.8305 0 0 0.6666667 0.6666667 0.6666667 1 132.8305 300.82742 0 0 0.6666667 0.6666667 0.6666667 1 153.1695 225.17258 0 0 0.6666667 0.6666667 0.6666667 1 158.82745 227.1695 0 0 0.6666667 0.6666667 0.6666667 1]
- indices: [0 1 2 1 2 3]
- dstRegion: {0 0 960 544}
- srcRegion: {0 0 1 1}
- subimageOffsets: [[0 0] [0 0] [0 0]]
- evenOdd: false
- canSkipMipmap: true
DrawTriangles done.
Internal image sizes:
  2: (1024, 512)
  3: (2778, 1284)
  6: (8192, 1024)
  8: (1024, 512)
  17: (1024, 512)
[...]
-[MTLDebugRenderCommandEncoder setScissorRect:]:3814: failed assertion `Set Scissor Rect Validation
(rect.y(0) + rect.height(544))(544) must be <= render pass height(512)
'
(lldb) 

@divVerent
Copy link
Contributor Author

dstRegion quite certainly oversteps i.image's bounds.

@divVerent
Copy link
Contributor Author

divVerent commented Jun 23, 2023

dstRegion is computed by taking initial dstRegion shifted by i.region.X/Y, intersecting with 0/0/i.region.Width/i.region.Height, and doubling. At least that's how it LOOKS - there's an obvious bug there (subtraction of i.region.X/Y is missing when computing x1/y1).

That can't be all there is to the bug though, as the min() should still prevent the worst. Looking further.

@divVerent
Copy link
Contributor Author

OH, it also adds region.X for the upper bound. That too can't be right.

Yeah, I can see how dstRegion is wrong - let me see if fixing that is all it takes.

@divVerent
Copy link
Contributor Author

One issue is, I can sure write a fix, but as the wrong dstRegion is always bigger than the correct region, it may not be possible to cover this by a test case, UNLESS we can run a test case with MTLDebugRenderCommandEncoder.

@divVerent
Copy link
Contributor Author

Fix confirmed, sending PR.

divVerent added a commit to divVerent/ebiten that referenced this issue Jun 23, 2023
divVerent added a commit to divVerent/ebiten that referenced this issue Jun 23, 2023
@divVerent
Copy link
Contributor Author

I see ONE potential chance to make a test case: allocate an atlas with multiple textures, then DrawTriangles on one and ReadPixels the others. I will try, but not sure if it will work. Plus, the test would be highly dependent on how atlas texture allocation work. Don't quite like that.

@divVerent
Copy link
Contributor Author

divVerent commented Jun 23, 2023

When I test on macOS, the temp texture AntiAlias uses never ends up on the same atlas as my NewImage textures, so I can never observe an out-of-bounds write attempt.

@hajimehoshi hajimehoshi changed the title failed assertion `Set Scissor Rect Validation internal/ui: a wrong calculation of a region for anti-aliasing Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants