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

Consider Extracting RenderTransform From DrawDevice #594

Closed
ilexp opened this issue Dec 22, 2017 · 3 comments
Closed

Consider Extracting RenderTransform From DrawDevice #594

ilexp opened this issue Dec 22, 2017 · 3 comments
Assignees
Labels
Breaking Change Breaks binary or source compatibility Cleanup Improving form, keeping function Core Area: Duality runtime or launcher Feature It doesn't exist yet, but I want it Nice2Have Beneficial, but only very slightly so Rendering Related to rendering / graphics Unit Tests Good candidate for adding more tests
Milestone

Comments

@ilexp
Copy link
Member

ilexp commented Dec 22, 2017

Summary

Over the course of issue #219 it became apparent that it might be useful to separate render matrix handling and API from actual drawing API. That way, Camera could internally have a general RenderTransform for coordinate transformation, and a DrawDevice for the actual rendering - instead of two DrawDevice instances with all the overhead.

Analysis

  • The name of the new class is not final. Consider other alternatives and find the best.
  • DrawDevice would internally keep a RenderTransform instance and expose it via property.
  • All of the transform API related DrawDevice code could be removed from DrawDevice entirely. This would clean it up a lot.
  • Other related transform API code in CamViewClient and similar could be replaced with a new getter property as well.
  • More advanced transform functionality would only need to be implemented in the new RenderTransform class, not spread out over various classes and APIs.
@ilexp ilexp added Cleanup Improving form, keeping function Core Area: Duality runtime or launcher Feature It doesn't exist yet, but I want it Nice2Have Beneficial, but only very slightly so Rendering Related to rendering / graphics Unit Tests Good candidate for adding more tests labels Dec 22, 2017
@ilexp ilexp added this to the v3.0 milestone Dec 22, 2017
@ilexp ilexp added the Breaking Change Breaks binary or source compatibility label Dec 22, 2017
@ilexp ilexp self-assigned this Feb 2, 2018
@ilexp
Copy link
Member Author

ilexp commented Feb 2, 2018

Progress

  • Created the new feature/3.0/rendertransform branch to work on this.
  • Set up a RenderTransform class by copying the relevant DrawDevice parts.

Immediate ToDo

  • Remove RenderMode from RenderTransform entirely, solve through configuration instead.
    • Actually, consider replacing it entirely with a third ProjectionMode.
  • Write unit tests for RenderTransform.
    • Move existing transform related DrawDevice tests.
    • Write more tests where appropriate.
  • Replace all related DrawDevice code with forwarding to an internal RenderTransform instance.
  • Replace public DrawDevice API with exposing the internal RenderTransform instance where it makes sense. Adjust IDrawDevice as well as the sites that use it accordingly.
  • Replace the internal transformDevice in Camera with a RenderTransform instance.

@ilexp
Copy link
Member Author

ilexp commented Feb 3, 2018

Progress

  • Merged RenderMode into ProjectionMode by adding Screen projection in addition to Orthographic and Perspective.
  • Removed RenderMode dependent code from the graphics backend and replaced it with additional rendering options for de/activating depth testing and depth writing.
  • Removed DrawDevice.DepthWrite property, as this should be considered an implementation detail by all renderers.

Immediate ToDo

  • Replace all related DrawDevice code with forwarding to an internal RenderTransform instance.
  • Replace public DrawDevice API with exposing the internal RenderTransform instance where it makes sense. Adjust IDrawDevice as well as the sites that use it accordingly.
  • Replace the internal transformDevice in Camera with a RenderTransform instance.
  • Write documentation for RenderTransform, XML comments for at least all public API.
  • Write unit tests for RenderTransform.
    • Move existing transform related DrawDevice tests.
    • Write more tests where appropriate.

@ilexp
Copy link
Member Author

ilexp commented Feb 3, 2018

Progress

  • Proceeded to extract all RenderTransform code from DrawDevice and started to refactor, but found the overall design somewhat unwieldy and inconvenient. Also, having to use some rendering properties on a sub-object of DrawDevice, but not others, felt a bit artificial. Decided to revert the main work on this issue, but kept some changes (like RenderMode and DepthWrite) that occurred along the way.
  • Added RenderOptions properties for depth testing and depth writing.

Closing this.

@ilexp ilexp closed this as completed Feb 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Breaks binary or source compatibility Cleanup Improving form, keeping function Core Area: Duality runtime or launcher Feature It doesn't exist yet, but I want it Nice2Have Beneficial, but only very slightly so Rendering Related to rendering / graphics Unit Tests Good candidate for adding more tests
Development

No branches or pull requests

1 participant