Add Multisample Anti-aliasing support #285
Conversation
|
Question: does this work with canvases? |
Canvas is just a go abstraction for a gl target i think? and this is implemented at the opengl level, so I'm not 100% sure, but I think it should. |
| @@ -163,6 +168,7 @@ func NewWindow(cfg WindowConfig) (*Window, error) { | |||
| // enter the OpenGL context | |||
| w.begin() | |||
| glhf.Init() | |||
| gl.Enable(gl.MULTISAMPLE) | |||
There was a problem hiding this comment.
Does having this enabled with a cfg.SamplesMSAA == 0 have any effect? If so, do we need a condition around this toggle?
There was a problem hiding this comment.
when the window hint for sampels is set to 0 (aka cfg.SamplesMSAA == 0), this doesn't do anything. From my reading as well, this is almost always enabled automatically by the driver whether its in use or not, so there's no reason to have a conditional.
|
Last question, you mention that setting this to a power of 2 enables the multisampling, is it just a no-op if it's set to not-a-power-of-2? |
Not sure honestly? Maybe would be worth having some vars in the package with various presets. |
|
Yeah I think that would be a good idea, suggest some reasonable values and if someone wants to do something else, they can. |
|
@dusk125 I added a check in NewWindow that will error out if you don't put in 0, 2, 4, 8, or 16. |
| @@ -119,6 +124,17 @@ func NewWindow(cfg WindowConfig) (*Window, error) { | |||
|
|
|||
| w := &Window{bounds: cfg.Bounds, cursorVisible: true} | |||
|
|
|||
| flag := false | |||
There was a problem hiding this comment.
Could do
if cfg.SampelsMSAA % 2 != 0 {
return nil, fmt.Errorf(...)
}Not a big deal, but wouldn't have to loop and it's a bit more clear we're looking for powers of two.
There was a problem hiding this comment.
I thought about this solution but wouldn't this just check to see if its even?
There was a problem hiding this comment.
hahahaha yes it would.....brain fart. I think then, if there are specific values that can be set otherwise, it's an error, we should at least have those specific values documented.
pixelgl/window.go
Outdated
| @@ -75,6 +77,9 @@ type WindowConfig struct { | |||
| // Invisible specifies whether the window will be initially hidden. | |||
| // You can make the window visible later using Window.Show(). | |||
| Invisible bool | |||
|
|
|||
| //SamplesMSAA specifies the level of MSAA to be used. 0 to disable. | |||
There was a problem hiding this comment.
I would add something like 'must be a power of two' since we're erroring if it's not.
|
Seeing as there's so much discussion about powers of two and only accepting them and erroring if not, why not just not let the user set an invalid value - accept an int and left shift |
That would certainly clear up the power of two problem; however, I think having a specific set of values is clearer to someone who may not be as experienced. |
I don't think that's too much to ask of a user, nor too high a price to pay for the nicer API. I'm just one dude though, I can be out of touch. I was floating the idea to see if it had a wider appeal. |
This PR adds a new WindowConfig value: SamplesMSAA.
When this is set to a power of 2, it will enable MSAA for the window.