ebiten: support customizing the "screen" shader

[hajimehoshi]

Forked from #1772

[hajimehoshi]

With #1772, what about

// DefaultScreenShader is the default shader to render the screen.
var DefaultScreenShader *Shader

// SetScreenShader sets the shader used to render the screen.
// The initial value is DefaultScreenShader.
// If shader is nil, a simple shader with the nearest shader is used.
func SetScreenShader(shader *Shader)

// Deprecated: as of v2.5. Use SetScreenShader instead.
func SetScreenFilterEnabled(enabled bool)

? /CC @divVerent

[hajimehoshi]

If there is SetScreenShader, don't we need a substitution of SetScreenFilterEnabled?

// SetScreenShader sets the shader used to render the screen.
// If shader is nil, the default screen shader is used.
func SetScreenShader(shader *Shader)

// Deprecated: as of v2.5. There is no substitution.
func SetScreenFilterEnabled(enabled bool)

[hajimehoshi]

@divVerent Would it be ok to you if SetScreenFilterEnabled is deprecated without a simple substitution, but you can specify a shader at SetScreenShader?

[hajimehoshi]

For the API, I have not deterined how to pass a uniform variable... 🤔

[divVerent]

Would work for me; when doing so, please ideally provide an example of a nearest neighbor shader.

On Sun, Sep 11, 2022, 07:07 Hajime Hoshi @.***> wrote:

@divVerent https://github.com/divVerent Would it be ok to you if SetScreenFilterEnabled is deprecated without a simple substitution, but you can specify a shader at SetScreenShader?

— Reply to this email directly, view it on GitHub https://github.com/hajimehoshi/ebiten/issues/2046#issuecomment-1242940320, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5NMFSISKXVA7PH3FKJ5LV5W4OVANCNFSM5SJ2OZEA . You are receiving this because you were mentioned.Message ID: @.***>

[divVerent]

I would be OK without uniforms, given I already use text substitution (and thus recompiling) in other places to enable/disable optional parts.

Would be nice and more consistent with other shader use though.

On Sun, Sep 11, 2022, 07:11 Hajime Hoshi @.***> wrote:

For the API, I have not deterined how to pass a uniform variable... 🤔

— Reply to this email directly, view it on GitHub https://github.com/hajimehoshi/ebiten/issues/2046#issuecomment-1242940954, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5NMHORIFLQMK7XL55KZTV5W46XANCNFSM5SJ2OZEA . You are receiving this because you were mentioned.Message ID: @.***>

[hajimehoshi]

Another idea is to add a mode to use the raw screen image at Draw instead of an offscreen, so that you can use any regular rendering functions

[divVerent]

That would probably fit best in Ebitengine's design, but IIRC you said the problem is that the screen image does not support all a regular Image does.

I could live with that (in video capture mode I would probably need to inject an intermediate image, but I already do that anyway for other reasons).

So I wonder if the real way to do that would be to add a second optional Draw method to Game that takes a ScreenImage that does not have the full Image feature set, and call that instead of Draw if it exists.

[hajimehoshi]

That would probably fit best in Ebitengine's design, but IIRC you said the problem is that the screen image does not support all a regular Image does.

Ah yes, the screen image doesn't work a rendering source so this is different from other regular images. This should work as a rendering target, hopefully.

So I wonder if the real way to do that would be to add a second optional Draw method to Game that takes a ScreenImage that does not have the full Image feature set, and call that instead of Draw if it exists.

Hmm, this might make things complicated. We cannot change the Game interface. It is actually possible to do a hack by casting an interface to check the existence of a new method (Draw2, for example) though, but I don't think it worth adding a new separate function so far.

[hajimehoshi]

Another restriction of the screen image is that it doesn't work with WritePixels.

[divVerent]

Sounds good; SetScreenFilterEnabled would be fully replaced by SetScreenShader(nil) or SetScreenShader(DefaultScreenShader).

On Fri, Aug 26, 2022 at 11:57 AM Hajime Hoshi @.***> wrote:

If there is SetScreenShader, don't we need a substitute of SetScreenFilterEnabled?

— Reply to this email directly, view it on GitHub https://github.com/hajimehoshi/ebiten/issues/2046#issuecomment-1228667874, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5NMH54LXBPIKXQDIH2HTV3DSOFANCNFSM5SJ2OZEA . You are receiving this because you were mentioned.Message ID: @.***>

[hajimehoshi]

I think I'll go with SetScreenShader(nil) for the default shader as I don't want to add a new exposed variable DefaultScreenShader.

For SetScreenFilterEnabled(false), users would have to prepare a simple shader by themselves, but I think this is fine. I'll document this anyway.

[hajimehoshi]

On the other hand, if there are use cases of the default shader, it might be fine to expose DefaultScreenShader, but are there such use cases?

[divVerent]

There is one interesting fact/problem here... normally Ebitengine doesn't actually use the screen shader when downscaling.

When using a custom shader, it'd be nice if the custom shader could be always forced, so one can also e.g. use it for dynamic fullscreen effects in a game.

Yes, there's no API to give them uniforms, which would be really nice for that purpose, but one can still somewhat efficitently for some flashing or similar effects by simply precompiling a few shaders initially and switching the screen shader as needed.

BTW, is switching the shader from Draw() valid and will it apply to this Draw() call?

So basically, my preferred behavior would be:

• If shader is set to nil, use the current logic that dynamically toggles between two shaders, depending on whether upscaling is needed or not.
• If a custom shader is set, the given shader will ALWAYS be used.
• Changing the shader from Draw() is valid.

This BTW speaks strongly against providing the DefaultShader as a global - after all, there is no shader that behaves precisely like default.

As an alternative, a way to force the use of the configured screen shader could be nice. But I'd probably prefer "nil = default, anything else = Ebitengine's toggle logic", as after all, the toggle logic could in theory be implemented in Kage itself (doesn't mean Ebitengine has to).

[hajimehoshi]

There is one interesting fact/problem here... normally Ebitengine doesn't actually use the screen shader when downscaling. [...] This BTW speaks strongly against providing the DefaultShader as a global - after all, there is no shader that behaves precisely like default.

Ahh, true, so originally it is impossible to represent the current situation by one shader.

BTW, is switching the shader from Draw() valid and will it apply to this Draw() call?

Yes. So,

So basically, my preferred behavior would be:

• If shader is set to nil, use the current logic that dynamically toggles between two shaders, depending on whether upscaling is needed or not.
• If a custom shader is set, the given shader will ALWAYS be used.
• Changing the shader from Draw() is valid.

I think this suggestion makes sense.

As an alternative, a way to force the use of the configured screen shader could be nice. But I'd probably prefer "nil = default, anything else = Ebitengine's toggle logic", as after all, the toggle logic could in theory be implemented in Kage itself (doesn't mean Ebitengine has to).

I don't understand the 'anything else' part. If a shader is specified, the toggle logic is not used in your suggestion, right?

Yet another alternative we have already considered is to add a new optional Game function like ScreenDraw, whose argument is an ebiten.Image for the screen image. The problem is that ebiten.Image has a lot of restrictions (e.g. this can be used only as a rendering target of Draw* functions). With this, you can implement your own toggle logic by yourself.

[divVerent]

Yeah, that is the idea - I was wrong above, I meant nil = Ebiten's toggle logic, anything else = the given shader unconditionally.

Game code can of course implement its own toggle logic by checking window dimensions from Draw() or Layout().

And yeah, I kinda prefer the custom shader over the "special ebiten.Image that doesn't support all ops". Is cleaner, even though slightly less flexible.

[hajimehoshi]

OK I think I'll go with SetScreenShader way (nil = the default behavior, non-nil = the specified shader is used in any scales) so far. I'll revisit this tomorrow or later.

[hajimehoshi]

And yeah, I kinda prefer the custom shader over the "special ebiten.Image that doesn't support all ops". Is cleaner, even though slightly less flexible.

What if the screen is given as an interface?

type ScreenImage interface {
    DrawImage(src *ebiten.Image, op *DrawImageOptions)
    DrawTriangles(vertices []Vertex, indices []uint16, img *Image, options *DrawTrianglesOptions)
    DrawRectShader(src *ebiten.Image op *DrawRectShaderOptions)
    DrawTrianglesShader(vertices []Vertex, indices []uint16, shader *Shader, options *DrawTrianglesShaderOptions)
}

type Game interface {
    DrawScreen(screen ScreenImage, offscreen *ebiten.Image)
}

[divVerent]

That would work, but also is a bit odd. I'd rather make it a concrete opaque struct type with just those methods.

Am Mi., 12. Okt. 2022 um 12:11 Uhr schrieb Hajime Hoshi < @.***>:

And yeah, I kinda prefer the custom shader over the "special ebiten.Image that doesn't support all ops". Is cleaner, even though slightly less flexible.

What if the screen is given as an interface?

type ScreenImage interface { DrawImage(src ebiten.Image, op DrawImageOptions) DrawTriangles(vertices []Vertex, indices []uint16, img Image, options DrawTrianglesOptions) DrawRectShader(src ebiten.Image op DrawRectShaderOptions) DrawTrianglesShader(vertices []Vertex, indices []uint16, shader Shader, options DrawTrianglesShaderOptions) } type Game interface { DrawScreen(screen ScreenImage) }

— Reply to this email directly, view it on GitHub https://github.com/hajimehoshi/ebiten/issues/2046#issuecomment-1276413363, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5NMD7WZMSGDZTQNNFKLLWC3OO7ANCNFSM5SJ2OZEA . You are receiving this because you were mentioned.Message ID: @.***>

[hajimehoshi]

It's possible to have a struct, but I don't think there is a strong difference here. I'm fine with both.

The problem of SetScreenShader is that we cannot have uniform variables. The default shader used a uniform variable, and I realized it would be useless if we cannot have uniform variables. Now the deafult shader doesn't use uniform variables, but I still believe uniform variables are useful.

[divVerent]

Well, we could simply get uniforms by adding a second arg to SetScreenShader, or not?

On October 13, 2022 5:26:24 AM GMT+02:00, Hajime Hoshi @.***> wrote:

It's possible to have a struct, but I don't think there is a strong difference here. I'm fine with both.

The problem of SetScreenShader is that we cannot have uniform variables. The default shader used a uniform variable, and I realized it would be useless if we cannot have uniform variables. Now the deafult shader doesn't use uniform variables, but I still believe uniform variables are useful.

-- Reply to this email directly or view it on GitHub: https://github.com/hajimehoshi/ebiten/issues/2046#issuecomment-1276977441 You are receiving this because you were mentioned.

Message ID: @.***>

[hajimehoshi]

Well, we could simply get uniforms by adding a second arg to SetScreenShader, or not?

I don't think it is a good idea. If the variables are changed every frame, is SetScreenShader called every frame?

[divVerent]

That's what this would mean, yes; and SetScreenShader may internally avoid some work if it notices the shader is the same. Not much different from DrawRectShader.

Also, of course, we can't really change the Game interface - so if a game were to have a custom shader, it'd add an extra method which Ebitengine then would detect via a different interface in addition to Game by a runtime type check. That isn't a big issue though - yes, it'd make the Draw method unused and the Layout method mostly unused (it'd still influence mapping of mouse or touch coordinates, and would dictate the size of the offscreen image passed to DrawScreen that isn't really necessary, but that's it). BTW is passing the offscreen image to DrawScreen even useful? The game could just allocate its own of an arbitrary size when using DrawScreen, and then really just use Layout for pointer coordinate mapping.

The big advantage of the DrawScreen approach is that it can do more than just have a final upscale pass. For example, the final pass could use multiple images as source, which could enable some additional effects at a lower performance cost than now. I have no ideas what to do with this right now, but I do see it as an advantage. And of course, one could use multiple Draw calls right onto the screen image, meaning the final pass can also e.g. combine game content and a status bar, HUD or similar stuff.

So personally I have no strong preference - for me both seem good.

[hajimehoshi]

it'd make the Draw method unused

I thought that Draw would be still used with DrawScreen, so Draw would manipulate the offscreen and DrawScreen would manipulate the screen. As long as we aim to replace SetScreenFilterEnabled, this should be correct.

So If the user wants an additional effect, this should work, but if the user prioritizes performance, this might be problematic. Is that correct? Even if we go with the SetScreenFilter way, the offscreen would still be used.

Layout method mostly unused

I didn't realize this point. DrawScreen could replace Layout at the same time, maybe? (not completely because Layout determines the size of the offscreen). This depends on the above point: whether Draw is called or not with DrawScreen.

The big advantage of the DrawScreen approach is that it can do more than just have a final upscale pass. For example, the final pass could use multiple images as source, which could enable some additional effects at a lower performance cost than now. I have no ideas what to do with this right now, but I do see it as an advantage. And of course, one could use multiple Draw calls right onto the screen image, meaning the final pass can also e.g. combine game content and a status bar, HUD or similar stuff.

Yeah, DrawScreen way is more flexible.

[divVerent]

On Thu, Oct 13, 2022, 07:30 Hajime Hoshi @.***> wrote:

it'd make the Draw method unused

I thought that Draw would be still used with DrawScreen, so Draw would manipulate the offscreen and DrawScreen would manipulate the screen. As long as we aim to replace SetScreenFilterEnabled, this should be correct.

So If the user wants an additional effect, this should work, but if the user prioritizes performance, this might be problematic. Is that correct? Even if we go with the SetScreenFilter way, the offscreen would be still used.

Not that problematic - obviously even with what you say, the user can make an empty Draw function, disable Ebitengine's clearing and do all work from DrawScreen. The only efficiency loss would be an unused texture. Seems harmless to me.

Personally I would actually prefer using DrawScreen as an extension like you suggest and keep using Draw. Makes for cleaner code.

Layout method mostly unused

I didn't realize this point. DrawScreen could replace Layout at the same time!

The big advantage of the DrawScreen approach is that it can do more than just have a final upscale pass. For example, the final pass could use multiple images as source, which could enable some additional effects at a lower performance cost than now. I have no ideas what to do with this right now, but I do see it as an advantage. And of course, one could use multiple Draw calls right onto the screen image, meaning the final pass can also e.g. combine game content and a status bar, HUD or similar stuff.

Yeah, DrawScreen way is more flexible.

— Reply to this email directly, view it on GitHub https://github.com/hajimehoshi/ebiten/issues/2046#issuecomment-1277049373, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5NMD5CTUZF4YCPVER4YTWC6MXTANCNFSM5SJ2OZEA . You are receiving this because you were mentioned.Message ID: @.***>

[hajimehoshi]

If you keep using Draw, Layout is still used to determine the offscreen size, right?

[divVerent]

Precisely. So let's go with that.

On Thu, Oct 13, 2022, 08:37 Hajime Hoshi @.***> wrote:

If you keep using Draw, Layout is still used to determine the offscreen size, right?

— Reply to this email directly, view it on GitHub https://github.com/hajimehoshi/ebiten/issues/2046#issuecomment-1277101097, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5NMHAAHS4K2NW63ASD33WC6U35ANCNFSM5SJ2OZEA . You are receiving this because you were mentioned.Message ID: @.***>

[divVerent]

This looks nice - one thing is a bit unfortunate, and that is that the aspect ratio keeping is the game's job. I.e. the game can't just scale the offscreen onto the screen, it should also center it keeping the aspect ratio.

I wonder if the API would be nicer if screen were already a same-aspect subrectangle. Not needed though - but this requirement may be worth documenting, especially as pointer coordinate mapping assumes this too.

Making ScreenScaleAndOffsets() a publicly available API would be a nice option though.

[hajimehoshi]

Sure, let me consider.

[divVerent]

I think I would prefer the documentation way, possibly with exporting that scale factor function, as the SubImage way is less flexible (e.g. having the whole screen allows influencing the color of the letterboxing).

On Fri, Oct 14, 2022, 08:24 Hajime Hoshi @.***> wrote:

Sure, let me consider.

— Reply to this email directly, view it on GitHub https://github.com/hajimehoshi/ebiten/issues/2046#issuecomment-1278936130, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5NMD4VT3PGVG73JFEHXDWDFGGDANCNFSM5SJ2OZEA . You are receiving this because you were mentioned.Message ID: @.***>

[hajimehoshi]

SubImage would not be an option so far. SubImage returns image.Image and a user would mistakenly convert this to *ebiten.Image.

possibly with exporting that scale factor function

By the way, would it be better to enable to adjust inputting (e.g. mouse cursor positions)? Now the scale and the offset is used for adjusting inputtings too.

[divVerent]

That would work too, of course - I think I would prefer keeping the current upscaling approach and keeping input "cooked", i.e. translated to offscreen coordinates, as well.

Just means we probably should make it easy to implement the letterboxing. It isn't hard, but it might be ideal to just export the scaling and offset values, either via function or passing them to the new Draw function.

On Fri, Oct 14, 2022, 10:06 Hajime Hoshi @.***> wrote:

SubImage would not be an option so far. SubImage returns image.Image and a user would mistakenly convert this to *ebiten.Image.

possibly with exporting that scale factor function

By the way, would it be better to enable to adjust inputting (e.g. mouse cursor positions)? Now the scale and the offset is used for adjusting inputtings too.

— Reply to this email directly, view it on GitHub https://github.com/hajimehoshi/ebiten/issues/2046#issuecomment-1279052946, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5NMGDMIV426KXITDEYYTWDFRY5ANCNFSM5SJ2OZEA . You are receiving this because you were mentioned.Message ID: @.***>

[hajimehoshi]

I think passing the parameters via DrawFinalScreen is the best so far. I'll revisit this tomorrow. Thanks!

[divVerent]

Maybe in form of a GeoM that would just work for DrawImage and DrawRectShader?

On Fri, Oct 14, 2022, 10:13 Hajime Hoshi @.***> wrote:

I think passing the parameters via DrawFinalScreen is the best so far. I'll revisit this tomorrow. Thanks!

— Reply to this email directly, view it on GitHub https://github.com/hajimehoshi/ebiten/issues/2046#issuecomment-1279063521, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB5NMBCUZMMC7PAQENTYKTWDFSZFANCNFSM5SJ2OZEA . You are receiving this because you were mentioned.Message ID: @.***>

[hajimehoshi]

That makes sense!

[hajimehoshi]

OK I've implemented it