Skip to content

Conversation

@SaracenOne
Copy link
Member

This branch adds support for classic multisample antialiasing through the framebuffer method. While this type of antialiasing is performance intensive and won't be compatibile with any deferred rendering pipelines, if you're using a forward renderer and you've got the performance to spare, it will give you probably the best picture quality, vastly exceeding FXAA in my opinion. Screenshots for comparision:
https://galateaproject.files.wordpress.com/2015/11/no_aa.png
https://galateaproject.files.wordpress.com/2015/11/fxaa.png
https://galateaproject.files.wordpress.com/2015/11/msaa.png

Copy link

Choose a reason for hiding this comment

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

Please only use one exclamation mark.

@SaracenOne
Copy link
Member Author

Gramatically incorrect, yes, and it was a bad warning that I only had while trying to implement the feature and probably shouldn't have been included in the pull request, but these error messages are actually just following the convention of the other warnings for failing to create a non-multisampled framebuffer 'WARN_PRINT("Could not create framebuffer!!");' on line 4387, and there are similarly grammatically incorrect error messages like 'ERR_PRINT("HTTP Chunk len not in hex!!");' in line 446 of http_client.cpp and other parts of the codebase. Either way, yes, I think it may probably be worth resolving these grammar and spelling issues, but it's probably worth doing it as one big pull request.

@ghost
Copy link

ghost commented Dec 26, 2015

Ugh, terrifying. Grammatical errors are worse than code-based errors.

@reduz
Copy link
Member

reduz commented Dec 26, 2015

This patch is good, hope to give it a little more configurability before
merging
On Dec 26, 2015 18:02, "Stian Furu Øverbye" notifications@github.com
wrote:

Ugh, terrifying. Grammatical errors are worse than code-based errors.


Reply to this email directly or view it on GitHub
#3032 (comment).

@SaracenOne SaracenOne force-pushed the msaa branch 2 times, most recently from 7ab271c to b668eeb Compare January 6, 2016 03:09
@tapir
Copy link

tapir commented Jan 26, 2016

any love for 2.0?

@reduz
Copy link
Member

reduz commented Jan 26, 2016

no.. 3.0

On Tue, Jan 26, 2016 at 12:58 PM, Coşku Baş notifications@github.com
wrote:

any love for 2.0?


Reply to this email directly or view it on GitHub
#3032 (comment).

@akien-mga
Copy link
Member

Should we really wait for 3.0 for this? I understand that this patch might be made obsolete with the new renderer in 3.0, but if it's working fine in the meantime for 2.1, it might be worth having no?

@nunodonato
Copy link
Contributor

yes please!! My game will be released before 3.0, this is a must have! :)

@slapin
Copy link
Contributor

slapin commented Apr 26, 2016

@akien-mga merge this one please, we can always add features later.

@reduz
Copy link
Member

reduz commented Apr 26, 2016

We'll soon start an early branch for whathever the new renderer becomes so
we can start working on it in parallel, so we don't have to wait so much to
merge/implement all this stuff

On Mon, Apr 25, 2016 at 10:02 PM, Sergey Lapin notifications@github.com
wrote:

@akien-mga https://github.com/akien-mga merge this one please, we can
always add features later.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#3032 (comment)

@slapin
Copy link
Contributor

slapin commented Apr 26, 2016

I don't think such things require waiting, just why not just create branch
and merge, it is just a few clicks.

On Tue, Apr 26, 2016 at 5:46 AM, Juan Linietsky notifications@github.com
wrote:

We'll soon start an early branch for whathever the new renderer becomes so
we can start working on it in parallel, so we don't have to wait so much to
merge/implement all this stuff

On Mon, Apr 25, 2016 at 10:02 PM, Sergey Lapin notifications@github.com
wrote:

@akien-mga https://github.com/akien-mga merge this one please, we can
always add features later.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#3032 (comment)


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#3032 (comment)

@akien-mga
Copy link
Member

IMO as long as @SaracenOne brings in cool improvements to the GLES2 renderer without regressions, we shouldn't have second thoughts merging them, even if the development focus will be put on the GLES3 renderer asap. We don't have an ETA for the next renderer, so why not use the enhancement for the old one that are already PR'ed?

@SaracenOne
Copy link
Member Author

SaracenOne commented Apr 30, 2016

Thank you, obviously it would be great as many as my things merged as possible, but it's probably not so much my hill to die on for this particular feature since I consider it more of a asethetic fluff feature rather than something of immediate importance.

Regarding the new renderer, I'm actually hoping that a design direction which priotises 'refactoring' things more so than rewriting them would be seen as a viable direction for still moving forward with 'next-gen' features while still retaining legacy support with very little maintaince burden, something similar in design to the Ogre3D project which maintains a mostly graphics API agnostic renderer perhaps (there's already a precendant for that with the design of shader system). I'd be interested to know if the main development team would be interested in such a direction, since it's not something I wouldn't want jump into headlong unless others were on board, but after spending a fair amount of time in the renderer, it seems like a sensible future-proof direction.

I'd be interested in discussing further in the near future.

@slapin
Copy link
Contributor

slapin commented Apr 30, 2016

I think GLES2 will be with us fir a long time, as new devices are still
shipped with GLES2 drivers. Only flagship devices do have GLES3 support,
and none have vulkan. The industry is slow, and it always been, when not
forced.

On Sat, Apr 30, 2016 at 8:12 AM, SaracenOne notifications@github.com
wrote:

Thank you, obviously it would be great as many as my things merged as
possible, but it's probably not so much my hill to die on for this
particular feature since I consider it more of a asethetic fluff feature
rather than something of immediate importance.

Regarding the new renderer, I'm actually hoping that a design direction
which priotises 'refactoring' things more so than rewriting them would be
seen as a viable direction for still moving forward with 'next-gen'
features while still retaining legacy support with very little maintaince
burden, something similar in design to the Ogre3D project which maintains a
mostly graphics API agnostic renderer perhaps. I'd be interested to know if
the main development team would be interested in such a direction, since
it's not something I would want jump into headlong unless others were on
board, but after spending a fair amount of time in the renderer, it seems
like a sensible future-proof direction.

I'd be interested in discussing further in the near future.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#3032 (comment)

@slapin
Copy link
Contributor

slapin commented Apr 30, 2016

I also agree that we need to merge as much as possible, and eventually
we'll hit 3.0

On Sat, Apr 30, 2016 at 9:05 AM, Sergey Lapin slapinid@gmail.com wrote:

I think GLES2 will be with us fir a long time, as new devices are still
shipped with GLES2 drivers. Only flagship devices do have GLES3 support,
and none have vulkan. The industry is slow, and it always been, when not
forced.

On Sat, Apr 30, 2016 at 8:12 AM, SaracenOne notifications@github.com
wrote:

Thank you, obviously it would be great as many as my things merged as
possible, but it's probably not so much my hill to die on for this
particular feature since I consider it more of a asethetic fluff feature
rather than something of immediate importance.

Regarding the new renderer, I'm actually hoping that a design direction
which priotises 'refactoring' things more so than rewriting them would be
seen as a viable direction for still moving forward with 'next-gen'
features while still retaining legacy support with very little maintaince
burden, something similar in design to the Ogre3D project which maintains a
mostly graphics API agnostic renderer perhaps. I'd be interested to know if
the main development team would be interested in such a direction, since
it's not something I would want jump into headlong unless others were on
board, but after spending a fair amount of time in the renderer, it seems
like a sensible future-proof direction.

I'd be interested in discussing further in the near future.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#3032 (comment)

@reduz
Copy link
Member

reduz commented Apr 30, 2016

@SaracenOne @slapin The plan is more just deprecating old stuff. Even if GLES2 support will be retained, I'm not sure it will be compatible with the current renderer, as it will most likely be only PBR based.

Of course the idea is to not force users working on current projects to rewrite everything, so instead of keeping more code in Godot for compatibility, I would rather we work in a migration tool that makes converting a project to 3.0 as easy and as automatic as possible (and will need your help for that).

@nunodonato
Copy link
Contributor

so... in the meantime, can this be merged? pretty please? :)

@akien-mga
Copy link
Member

For the reference, with the focus shifting towards the GLES3 renderer for real now, this PR likely won't be merged (or it should have been merged 6 months ago like many advocated).

But keeping it open as a reference implementation for @reduz to check when working on the new renderer.

@reduz
Copy link
Member

reduz commented Jan 11, 2017

3.0 already has MSAA support, so closing this

@reduz reduz closed this Jan 11, 2017
@chaigler
Copy link

chaigler commented Nov 2, 2017

Could we reconsider this, please? 3.0 is still a ways out from being production ready, meanwhile 2.x is suitable for a wide range of 3D games (while maintaining great compatibility across devices) but suffers from poor AA due to the FXAA implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants