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

WIP: MRT Proof of concept #2953

Closed
wants to merge 48 commits into from
Closed

WIP: MRT Proof of concept #2953

wants to merge 48 commits into from

Conversation

Zyko0
Copy link
Contributor

@Zyko0 Zyko0 commented Apr 6, 2024

What issue is this addressing?

#2930

What this PR does | solves

This is a proof of concept of MRT (multiple render target), to investigate if the concept could be integrated to ebitengine, this is not a merge candidate for now.

go run ./examples/mrt
image

dsts = [4]*ebiten.Image{
		ebiten.NewImageWithOptions(image.Rect(0, 0, 128, 128), &ebiten.NewImageOptions{Unmanaged: true}),
		ebiten.NewImageWithOptions(image.Rect(0, 0, 128, 128), &ebiten.NewImageOptions{Unmanaged: true}),
		ebiten.NewImageWithOptions(image.Rect(0, 0, 128, 128), &ebiten.NewImageOptions{Unmanaged: true}),
		ebiten.NewImageWithOptions(image.Rect(0, 0, 128, 128), &ebiten.NewImageOptions{Unmanaged: true}),
}
shaderSrc = []byte(`
//kage:units pixels

package main

func Fragment(dst vec4, src vec2, color vec4) (vec4, vec4, vec4, vec4) {
	return vec4(1,0,0,1), vec4(0,1,0,1), vec4(0,0,1,1), vec4(1,0,1,1)
}
`

{
      ebiten.DrawTrianglesShaderMRT(dsts, vertices, indices, s, nil)
      // screen.Draw all 4 images to assess that they've been written to with the same geometry / draw call
}
  • OpenGL
  • DirectX 11
  • DirectX 12
  • WebGL => A bit more problematic due to the glsl compatibility between desktop
  • Metal

Notes:

  • It only seems to make sense with unmanaged *ebiten.Image (as in different internal textures).
    • dst images could be moved to separate atlases if they're not ebiten.unmanaged: Support Multiple render target (MRT) #2930 (comment)
    • dst images, if moved to new textures or existing (available) ones should share the same region (rectangle) as they originally do, because of scissor, viewport definition but mostly because the triangles define the logical destination position, moving to an arbitrary region would make the triangles target the wrong rectangle
    • dst images should have the same sizes probably, since the triangles, scissor and viewport should target the same region for all destination textures
  • I set the max dst images count to 8, since 8 seems to be the guaranteed minimum by all drivers
  • The example found in examples/mrt is probably to be removed and changed for a more relevant example, I put it there so that we can see a minimal reproduction of the feature temporarily

@Zyko0 Zyko0 mentioned this pull request Apr 6, 2024
11 tasks
@Zyko0 Zyko0 force-pushed the main branch 3 times, most recently from bc1eae0 to 5b54a1d Compare April 13, 2024 14:21
examples/mrt/main.go Outdated Show resolved Hide resolved
@hajimehoshi
Copy link
Owner

hajimehoshi commented Jul 8, 2024

Please update your PR for the latest main branch, thanks!

@@ -440,6 +440,27 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderSrcImageCount]*Image, vertice
i.drawTriangles(srcs, vertices, indices, blend, dstRegion, srcRegions, shader, uniforms, fillRule)
}

func DrawTrianglesMRT(dsts [graphics.ShaderDstImageCount]*Image, srcs [graphics.ShaderSrcImageCount]*Image, vertices []float32, indices []uint32, blend graphicsdriver.Blend, dstRegion image.Rectangle, srcRegions [graphics.ShaderSrcImageCount]image.Rectangle, shader *Shader, uniforms []uint32, fillRule graphicsdriver.FillRule) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why not unifying DrawTriangles with this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally thought I might have a lot of additional things to write and wanted to make it distinct, but I can merge them now!
Another point is that I was afraid of the cost of the copy of an extra array of 8 pointers in all paths with (5-6x layer depth, ebiten,ui,buffered,mipmap,atlas, etc..) which would apply to all other standard two-triangles DrawImage call.

Copy link
Owner

Choose a reason for hiding this comment

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

Another point is that I was afraid of the cost of the copy of an extra array of 8 pointers in all paths with (5-6x layer depth, ebiten,ui,buffered,mipmap,atlas, etc..) which would apply to all other standard two-triangles DrawImage call.

I don't think this would be problematic in terms of performance. Also, I'm not sure we really want 8 images (4 would be enough for example?).

// the value is kept and is not clamped.
//
// When the image i is disposed, DrawTrianglesShader does nothing.
func DrawTrianglesShaderMRT(dsts [graphics.ShaderDstImageCount]*Image, vertices []Vertex, indices []uint16, shader *Shader, options *DrawTrianglesShaderOptions) {
Copy link
Owner

Choose a reason for hiding this comment

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

We should be able to share most of implementations with DrawTriangles

@@ -211,6 +211,51 @@ func (i *Image) DrawTriangles(srcs [graphics.ShaderSrcImageCount]*Image, vertice
i.pixels = nil
}

func DrawTrianglesMRT(dsts [graphics.ShaderDstImageCount]*Image, srcs [graphics.ShaderSrcImageCount]*Image, vertices []float32, indices []uint32, blend graphicsdriver.Blend, dstRegion image.Rectangle, srcRegions [graphics.ShaderSrcImageCount]image.Rectangle, shader *atlas.Shader, uniforms []uint32, fillRule graphicsdriver.FillRule) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why not unifying DrawTriangles with this?

//
// If the source image is not specified, i.e., src is nil and there is no image in the uniform variables, the
// elements for the source image are not used.
func DrawTrianglesMRT(dsts [graphics.ShaderDstImageCount]*Image, srcs [graphics.ShaderSrcImageCount]*Image, vertices []float32, indices []uint32, blend graphicsdriver.Blend, dstRegion image.Rectangle, srcRegions [graphics.ShaderSrcImageCount]image.Rectangle, shader *Shader, uniforms []uint32, fillRule graphicsdriver.FillRule) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why not unifying DrawTriangles with this?

@Zyko0
Copy link
Contributor Author

Zyko0 commented Jul 16, 2024

Let's close this PR, since we require more 2D use cases and user requests regarding this feature: #2930 (comment)

@Zyko0 Zyko0 closed this Jul 16, 2024
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.

2 participants