Ticket #9085 (reopened enhancement)

Opened 2 years ago

Last modified 18 months ago

[2.8] Full Scene Anti Aliasing for wxGLCanvas.

Reported by: olivier_playez Owned by:
Priority: normal Milestone:
Component: OpenGL Version: 2.8.9
Keywords: Cc: olivier_playez, g00fy
Blocked By: Patch: yes
Blocking:

Description

This patch adds the support for Full Scene Anti Aliasing (FSAA) to the wxGLCanvas widget (using the multisample extension).
It works on mac/carbon, gtk and msw.

It adds two flags (namely WX_GL_SAMPLE_BUFFERS and WX_GL_SAMPLES) allowing for 2x2 or 4x4 hardware accelerated full scene anti aliasing on most recent graphics cards (including nVidia and ATI).

The documentation was updated accordingly.


You might remember this description, it's from the patch [ 1844855 ] Full Scene Anti Aliasing (FSAA) for wxGLCanvas.

I rewrote the patch because it failed to create a valid context if multisample is requested on a card that doesn't support it. It's very platform specific to query for multisample support, so it has to be included in wxGLCanvas.
With this new patch, if multisample is requested but not available, the wxGLCanvas will be created without multisample. You can use wxGLCanvas::IsDisplaySupported() to check if the display you requested is supported (and decide to quit or continue).

I added multisample support in the penguin sample.

This patch is for the following snapshot  http://biolpc22.york.ac.uk/pub/Daily_HEAD/files/wxWidgets-2008-03-15.tar.gz

Anti Aliasing is an important feature. As you can read in the nVidia GPU programming guide ( http://developer.download.nvidia.com/GPU_Programming_Guide/GPU_Programming_Guide.pdf), page 31 :
"GeForce FX, GeForce 6 Series, and GeForce 7 Series GPUs all have powerful antialiasing engines. They perform best with antialiasing enabled, so we recommend that you enable your applications for antialiasing."

The multisample extension is supported by most graphics cards, here is a list :
 http://www.delphi3d.net/hardware/extsupport.php?extension=GL_ARB_multisample

I tested this patch on :
- WinXP with a GeForce FX 5200 (supports FSAA)
- WinXP with an i915 (doesn't support FSAA).
- Mac OS X 10.4.11 with an ATI Rage 128 (doesn't support FSAA).
- Xubuntu 7.10 with a GeForce FX 5200 (supports FSAA, with nvidia drivers).

Attachments

GLCanvasFSAA.patch download (26.0 KB) - added by olivier_playez 2 years ago.
Full Scene Anti Aliasing (using multisample) for wxGLCanvas
GLCanvasFSAA_287.patch download (27.5 KB) - added by olivier_playez 2 years ago.
Full Scene Anti Aliasing (using multisample) for wxGLCanvas (backport for v2.8.7)
GLCanvasFSAAv2_GLXfix_noIDS.patch download (23.7 KB) - added by olivier_playez 2 years ago.
Full Scene Anti Aliasing (using multisample) for wxGLCanvas v2
GLCanvasFSAA_287v2_GLXfix.patch download (28.1 KB) - added by olivier_playez 2 years ago.
Full Scene Anti Aliasing (using multisample) for wxGLCanvas v2 (backport for v2.8.7)
GLCanvasFSAA_289v3.patch download (26.7 KB) - added by olivier.playez 18 months ago.
I've updated the patch for wxWidgets 2.8.9 and fixed an issue in the macos version

Change History

Changed 2 years ago by olivier_playez

Full Scene Anti Aliasing (using multisample) for wxGLCanvas

Changed 2 years ago by olivier_playez

Full Scene Anti Aliasing (using multisample) for wxGLCanvas (backport for v2.8.7)

  Changed 2 years ago by olivier_playez

File Added: GLCanvasFSAA_287.patch

  Changed 2 years ago by vadz

The patch looks globally good, thanks, but I have several questions:

1. Would it be possible to put IsExtensionSupported() in wxGLCanvasBase instead of duplicating it in all ports? We really want to avoid having to maintain the same code in several places.

2. The logic behind pixelFormatMultiSample in wxMSW version is not really clear, could you please describe it in a comment somewhere (e.g. what values it can have and what do they mean)? Also, please call it ms_pixelFormatMultiSample to conform to wx convention. And, finally, the trivial accessor to it doesn't seem to be necessary as most of the code just uses it directly anyhow (OTOH an accessor would be helpful if it's initialized on demand...). And if this could be somehow simplified it would be great.

3. A comment about the meaning of "fullTest" parameter would be appreciated too, it's really not clear what does it mean.

4. In Unix and Mac versions it doesn't seem to be necessary to always call IsExtensionSupported(wxT("GL_ARB_multisample"), it's only needed when WX_GL_SAMPLE[_BUFFERS] is specified, isn't it?

5. The MSW code defines tons of WGL_XXX_ARB constants only a few of which are used in the code, could we cut down on the unneeded ones?

Thanks in advance!

Changed 2 years ago by olivier_playez

Full Scene Anti Aliasing (using multisample) for wxGLCanvas v2

  Changed 2 years ago by olivier_playez

1. done

2. pixelFormatMultiSample has been renamed and commented in the header file.

2. trivial accessor and 3. : it was used for the IsDisplaySupported() feature introduced by the patch 1879906. I removed them in this new version of patch, support for this feature should be added later, preferably by it's author.

4. fixed

5. unused WGL_XXX_ARB constants have been removed

File Added: GLCanvasFSAAv2_GLXfix_noIDS.patch

Changed 2 years ago by olivier_playez

  Changed 2 years ago by olivier_playez

I forgot to say I fixed an issue in the unix version (for the trunk and v2.8.7)
File Added: GLCanvasFSAA_287v2_GLXfix.patch

Changed 2 years ago by olivier_playez

Full Scene Anti Aliasing (using multisample) for wxGLCanvas v2 (backport for v2.8.7)

  Changed 2 years ago by g00fy

Hi,

I converted this patch (at least for MSW) to trunk.
It would be nice to see this included into wx.

Please find it here:
 https://sourceforge.net/tracker/index.php?func=detail&aid=1956090&group_id=9863&atid=309863

Greetings

  Changed 2 years ago by vadz

Thanks, I'll look at the trunk version first.

  Changed 2 years ago by g00fy

I updated my patch for trunk in the previously mentioned Patches-thread.

Olivier, could you do me a favor and check it if you think it's correct?

Thanks!

  Changed 2 years ago by wojdyr

  • type set to enhancement

  Changed 2 years ago by olivier_playez

The priority for this patch has been set to 'low', so I would like to know when is the best time to update this patch for the trunk. Is there a chance it will be included in v3.0.0 ?
I would like to see this patch included in v2.8.8

follow-up: ↓ 12   Changed 2 years ago by vadz

  • cc vadz removed
  • priority changed from low to normal
  • status changed from new to confirmed

I don't think it should be low in this tracker, I had set it to be slightly lower than normal to indicate that the patch to the trunk (#9145) had to be applied first.

I don't remember this patch in the details to be honest but I don't think it breaks ABI so there shouldn't be any problem in including it in a next 2.8 release although I'm afraid it will have to be 2.8.9 as we simply don't have enough time to test it before 2.8.8 which is imminent any more.

Just one thing from this point of view: could you please add

#if wxABI_VERSION >= 20809
...
#endif

around all new public symbols (e.g. attribute names) added by this patch?

And, last, a question: do I understand correctly that only the last of these patches is meant to be applied and the other ones are just previous versions?

  Changed 2 years ago by olivier_playez

There is two versions of this patch : v2.8.7 and the trunk (3 months old now). I've been using this patch with v2.8.7 for 3 months, on mac and windows, and there's no bug report so far.
I'll make the changes for the v2.8 as soon as possible (it takes time to test on mac/win/linux).

in reply to: ↑ 10   Changed 2 years ago by olivier_playez

Replying to vadz:

And, last, a question: do I understand correctly that only the last of these patches is meant to be applied and the other ones are just previous versions?

Yes, ignore the files GLCanvasFSAA.patch and GLCanvasFSAA_287.patch.
Use GLCanvasFSAAv2_GLXfix_noIDS.patch and GLCanvasFSAA_287v2_GLXfix.patch

  Changed 2 years ago by vadz

  • status changed from confirmed to closed
  • resolution set to outdated

I think this patch needs to be updated now to be more like what we have in the trunk (normally there shouldn't be any problems as ABI shouldn't be affected by these changes). It's also probably worth to wait a bit to see if any problems are found with the trunk version, so I'm closing this report in the meanwhile as I don't want to apply this patch as is.

Please feel free to reopen it or create another one if you decide to backport trunk changes to 2.8.

Thanks!

Changed 18 months ago by olivier.playez

I've updated the patch for wxWidgets 2.8.9 and fixed an issue in the macos version

  Changed 18 months ago by olivier.playez

  • status changed from closed to reopened
  • version set to 2.8.9
  • resolution deleted
Note: See TracTickets for help on using tickets.