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

Get Rid of Deprecated OpenGL 2.1 Functionality #220

Closed
ilexp opened this issue Oct 9, 2015 · 10 comments
Closed

Get Rid of Deprecated OpenGL 2.1 Functionality #220

ilexp opened this issue Oct 9, 2015 · 10 comments
Assignees
Labels
Backend Area: Backend core / editor plugins (OpenTK, etc.) Breaking Change Breaks binary or source compatibility Cleanup Improving form, keeping function Core Area: Duality runtime or launcher Rendering Related to rendering / graphics Task ToDo that's neither a Bug, nor a Feature
Milestone

Comments

@ilexp
Copy link
Member

ilexp commented Oct 9, 2015

The default Duality backend still uses a lot of builtin / fixed function API in OpenGL 2.1. Get rid of it in order to prepare a future upgrade of the OpenGL version, as well as to provide a better sample implementation for OpenGL ES backends.

Things to address:

  • Don't use any builtin shader variables.
  • Don't use any pre-specified vertex attributes.
  • Switch namespaces and use OpenTK's ES API, rather than the regular GL API. (If possible) Edit: Doesn't work that way - OpenGL ES may not be available on desktop platforms even if they offer a superset of its features.
  • In general, don't use anything that isn't in GL ES 2.1 according to the docs.

This should probably be addressed in conjunction with the Vertex Transform change.

@ilexp ilexp added Task ToDo that's neither a Bug, nor a Feature Core Area: Duality runtime or launcher labels Oct 9, 2015
@ilexp ilexp added this to the The Future Is Now milestone Oct 9, 2015
@ilexp ilexp added the Backend Area: Backend core / editor plugins (OpenTK, etc.) label Oct 11, 2015
@ilexp ilexp added the Breaking Change Breaks binary or source compatibility label Nov 12, 2015
@ilexp ilexp modified the milestones: The Future Is Now, v3.0 Feb 13, 2016
@ilexp ilexp added the Cleanup Improving form, keeping function label Apr 30, 2016
@ilexp ilexp added the Rendering Related to rendering / graphics label Jul 3, 2016
@ilexp
Copy link
Member Author

ilexp commented Dec 22, 2017

Progress

  • All vertex transform now happens on the GPU, see issue Move Camera Vertex Transform to GPU #219.
  • Every rendering operation now uses a shader, fixed function rendering is no longer supported.
  • Various improvements to shader parameter handling and builtin shader functions.

Immediate ToDo

  • Identify all non-GLES 2.0 OpenGL interaction.
  • Replace all OpenGL interaction with a GLES 2.0 equivalent.
  • Cleanup code and remove all that was supporting fixed function rendering.

@ilexp
Copy link
Member Author

ilexp commented Jan 6, 2018

Progress

  • Investigated which parts of the OpenGL backend are deprecated OpenGL 2.1.
  • Found that the editor graphics backend only does context / window management, but not a single GL call, so it can be omitted from this issue entirely.
  • Found that not all functionality in the graphics backend can be expressed as OpenGL ES 2.0 code:
    • Blitting Framebuffers (Multisampling RenderTargets)
    • Float-based pixel data formats.
  • Removed support for the BGRA pixel format, as it's currently unused and unsupported by GLES 2.0.

Immediate ToDo

  • Replace deprecated OpenGL 2.1 code with modern equivalents:
    • Use custom model, view and projection matrix uniforms instead of OpenGL builtin ones.
      • Remove: GL.MatrixMode, GL.LoadMatrix, GL.Scale, GL.Translate
    • Use custom vertex attributes instead of OpenGL builtin ones.
      • Remove: GL.EnableClientState, GL.VertexPointer, GL.TexCoordPointer, GL.ColorPointer
      • Look through all shader code to remove builtin varyings as well.
    • Implement alpha test as fragment shader code instead of relying on OpenGL builtin alpha testing.
      • Remove: GL.AlphaFunc, EnableCap.AlphaTest
      • Alpha testing will require some sort of a backend internal shared-but-per-material uniform in order to switch it on or off dynamically. This can not be solved with custom shaders on the user side, as the Mask blendmode decides whether to use alpha testing or alpha to coverage dynamically.
      • Keep this internal, no "public API feature" for this required or desired.

@ilexp
Copy link
Member Author

ilexp commented Jan 6, 2018

Progress

  • Removed OpenGL builtin matrix stack with a custom one consisting of view, projection and view*projection matrices.
  • Introduced conditional symbol support to ShaderSourceBuilder, and used them to distinguish shader types via preprocessor definition in order to allow builtin shader functions that are only available to either fragment or vertex shaders.

Immediate ToDo

  • Use custom vertex attributes instead of OpenGL builtin ones.
    • Remove: GL.EnableClientState, GL.VertexPointer, GL.TexCoordPointer, GL.ColorPointer
    • Look through all shader code to remove builtin varyings as well.
  • Implement alpha test as fragment shader code instead of relying on OpenGL builtin alpha testing.
    • Remove: GL.AlphaFunc, EnableCap.AlphaTest
    • Alpha testing will require some sort of a backend internal shared-but-per-material uniform in order to switch it on or off dynamically. This can not be solved with custom shaders on the user side, as the Mask blendmode decides whether to use alpha testing or alpha to coverage dynamically.
    • Keep this internal, no "public API feature" for this required or desired.

@ilexp
Copy link
Member Author

ilexp commented Jan 7, 2018

Progress

  • Adopted a consistent "camelCase" style for all uniforms, attributes and varyings in shader code.
  • Replaced OpenGL alpha testing with custom, shader-based alpha testing.
  • Added an "internal shader state" parameter collection in the graphics backend, which is used as a fallback for all otherwise undefined parameters. It is currently used only for the dynamic alpha testing threshold value, but can be used for other dynamically calculated per-material parameters as well.

Immediate ToDo

  • Use custom vertex attributes instead of OpenGL builtin ones.
    • Remove: GL.EnableClientState, GL.VertexPointer, GL.TexCoordPointer, GL.ColorPointer
    • Look through all shader code to remove builtin varyings as well.
  • Do a test run with a modern OpenGL context to see if everything runs fine.
  • If it does, consider moving towards a non-legacy OpenGL context version.

@ilexp
Copy link
Member Author

ilexp commented Jan 17, 2018

Progress

  • Adjusted all builtin and sample project shaders to explicitly define all vertex attributes and varyings. No longer using any legacy OpenGL builtin variables.
  • Removed VertexElement roles and backend support for builtin OpenGL vertex attributes.

Immediate ToDo

  • Consider switching to GLSL 1.3 / OpenGL 3.0 by default.
    • OpenGL 3.0 has been out since 2008 and is supported even by most Intel on-chip GPU hardware except first generation Intel HD.
    • Replace all shader varyings with in / out declarations.
    • Update all builtin and sample shaders: CustomRenderSetup, BasicShaders, DualStickSpaceShooter, SmoothAnimation, DynamicLighting.

@ilexp
Copy link
Member Author

ilexp commented Jan 18, 2018

Progress

  • Did a quick OpenGL 3.0 experiment and fixed various shader related issues in the process. Also improved shader compiler logging. Stashed changes locally, as this was too rough to be committed at this point.
  • Found an issue where picking in the dynamic lighting sample doesn't work anymore. Something is off with the picking colors:
    picking

Immediate ToDo

  • Fix the picking bug in the dynamic lighting sample.
    • Note that it still seems to work in the regular editor test case.
    • It also seems to work when dropping a regular SpriteRenderer into the affected sample, even if it uses the affected material.
    • Could it have something to do with different vertex formats?
    • Could it be related to now using normalized vertex attributes?
  • Consider switching to GLSL 1.3 / OpenGL 3.0 by default.
    • OpenGL 3.0 has been out since 2008 and is supported even by most Intel on-chip GPU hardware except first generation Intel HD.
    • Replace all shader varyings with in / out declarations.
    • Update all builtin and sample shaders: CustomRenderSetup, BasicShaders, DualStickSpaceShooter, SmoothAnimation, DynamicLighting.

@ilexp
Copy link
Member Author

ilexp commented Jan 19, 2018

Progress

  • Fixed the picking bug, which turned out to be caused by legacy vertex element to shader field mapping code that mapped multiple elements to the same fields, in case not all elements had their own field. Fixed this by removing the legacy (by-type) mapping code.
  • Fixed a bug in GameObjectManager that cause constant redraws in the editor.
  • Found that the current GLSL 1.3 / OpenGL 3.0 work-in-progress stash doesn't seem to render as smoothly as the checked in develop-3.0 version.

Immediate ToDo

  • Investigate why rendering doesn't work as smoothly in the work-in-progress GLSL 1.3 / OpenGL 3.0 stash.
    • Move the CamView camera using the middle mouse button at constant speed and all is smooth. Release the middle mouse button and observe stuttering in the movement fade-out. It's only there when the stashed changes are applied.
    • Does the stuttering show up in frame timings?
    • Find out what change exactly triggers this.
  • Consider switching to GLSL 1.3 / OpenGL 3.0 by default.
    • OpenGL 3.0 has been out since 2008 and is supported even by most Intel on-chip GPU hardware except first generation Intel HD.
    • Replace all shader varyings with in / out declarations.
    • Update all builtin and sample shaders: CustomRenderSetup, BasicShaders, DualStickSpaceShooter, SmoothAnimation, DynamicLighting.

ilexp added a commit that referenced this issue Jan 20, 2018
#CHANGE: Now requiring OpenGL 3.0 as Duality minspec. It has been out for ten years by now and even most Intel on-chip GPUs seem to support it. For more info, see issue #220.
@ilexp
Copy link
Member Author

ilexp commented Jan 20, 2018

Progress

  • Found that the previously recorded stuttering / perf regression was due to a forgotten part of debug code that would save every frames picking result texture to a .png file on my desktop. Removed it and fixed the stuttering.
  • Updated all shaders to use GLSL 1.3 syntax.
  • Switched to OpenGL 3.0 as a minspec, reflecting the shader syntax change.
  • Removed usage of GL_QUADS primitive type and instead emulating it on the backend using an index buffer.

Immediate ToDo

  • Limit the number of vertices in a single vertex buffer to what can be indexed using a ushort to remain compatible with OpenGL ES 2.0. This will affect:
    • The upload code in BeginRendering, to split up vertices into multiple VBOs.
    • The drawing code in DrawVertexBatch, or its surrounding code, to select the proper VBO and adjust the range.
    • The GL_QUADS emulation code, since there can now be more than one VBO used in a single batch.

@ilexp
Copy link
Member Author

ilexp commented Jan 20, 2018

Progress

  • Found that backend only split-up of vertices into VBOs won't work easily with the current setup, where all vertices are in a shared storage. It needs to be possible to guarantee that a single AddVertices calls vertices remain in the same VBO, which is way easier with a core implementation than a backend implementation.
  • Switched the internal index buffer from ushort to uint indexing to guarantee full functionality for large vertex counts and remove urgency for limited size VBOs, so this can be done in issue Introduce Explicit Vertex Buffers to the Graphics Backend Interface #487.

Immediate ToDo

  • Prepare core code for introducing limited-size VBOs.
    • Extend VertexBatchStore to allow multiple buffers per type and do an internal auto-split if a buffer would otherwise exceed a max size. By default, use int.MaxValue.
    • Extend VertexDrawItem to carry a buffer index.
    • Extend DrawBatch to carry a buffer index.
    • Adjust batch aggregation to stop on differing buffer index.
    • Adjust GraphicsBackend to use multiple VBOs per type to reflect the above changes.
  • Limit to ushort size VBOs.
    • Change max size for VertexBatchStore instance in DrawDevice to ushort.MaxValue.
    • Adjust VertexDrawItem to use a ushort for both offset and count to save memory / increase throughput.
    • Adjust GraphicsBackend to use ushort indices.
  • Test thoroughly
    • Add unit tests for the new VertexBatchStore API and splitting behavior.
    • Add unit tests for DrawDevice to validate vertex input / batch output.
    • Artificially reduce the default max size to 1024 and run the benchmark and the tilemaps sample. Observe behavior and performance.

@ilexp
Copy link
Member Author

ilexp commented Jan 21, 2018

Progress

  • Adjusted DrawDevice and VertexBatchStore as described and added unit tests to check the new behavior and API.
  • Added support for multiple vertex buffers per vertex type to the graphics backend.
  • Switched to ushort indexing in both the graphics backend and DrawDevice internals.

Done. Closing this.

@ilexp ilexp closed this as completed Jan 21, 2018
@ilexp ilexp self-assigned this Jan 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend Area: Backend core / editor plugins (OpenTK, etc.) Breaking Change Breaks binary or source compatibility Cleanup Improving form, keeping function Core Area: Duality runtime or launcher Rendering Related to rendering / graphics Task ToDo that's neither a Bug, nor a Feature
Development

No branches or pull requests

1 participant