Graphics_oM: all Render* geometrical objects to extend base Geometry interfaces
- Dominant language
- C#
- Stars
- 247
- Forks
- 47
- Avg merge
- 7d 7h
- Merged PRs (30d)
- 4
Description
#### Description:
I often encountered issues when dealing with Graphics_oM objects due to code duplication. A good example is `RenderMesh`.
In `RenderMesh`, we declare 2 properties:
https://github.com/BHoM/BHoM/blob/e0b2a43a97b9687ac0bb3d0bf3a1bc682fe879c0/Graphics_oM/Render/RenderMesh.cs#L32-L50
Note that:
- `Vertices` is of type [`BH.oM.Graphics.Vertex`](https://github.com/BHoM/BHoM/blob/e0b2a43a97b9687ac0bb3d0bf3a1bc682fe879c0/Graphics_oM/Render/Vertex.cs#L28-L47).
- This is simply a class that owns a `BH.oM.Geometry.Point` and a `Colour`. However, this means that to access the point you need to do `Vertex.Point`.
- Because of the above, we are forced to add an `explicit casting`.
- The property `Faces` is of type `BH.oM.Geometry.Face`. The two properties are inconsistently coming from different namespaces and it becomes confusing.
- Despite the `explicit casting`, we still need [convert methods to deal between `Geometry.Mesh` and `RenderMesh`](https://github.com/BHoM/BHoM_Engine/blob/master/Graphics_Engine/Convert/ToRenderMesh.cs)
As we can see, this creates lots of troble. In particular:
- we have to manually convert between `Geometry.Mesh` and `RenderMesh`
- hence, all methods that could be applied to `Geometry.Mesh` are not directly applicable to `RenderMesh`
If we go forward with [creating more Graphics Render* objects](https://github.com/BHoM/BHoM/issues/1271), this problem will extend to them.
### Suggestion
We can think of mirroring the Geometry namespace, adding Render* objects (e.g. RenderLine, RenderMesh, etc...) by extending their respective Geometry types with additional Graphical properties. E.g. `RenderMesh : Mesh`
If we worry about inheritance madness, we can safely mark Render* classes as `sealed`.
For example, we can have:
Refactor of [`Vertex`](https://github.com/BHoM/BHoM/blob/e0b2a43a97b9687ac0bb3d0bf3a1bc682fe879c0/Graphics_oM/Render/Vertex.cs#L28-L47):
```cs
public sealed class RenderPoint : Geometry.Point, IColourable
{
/***************************************************/
/**** Properties ****/
/***************************************************/
public virtual Color Colour { get; set; } = Color.FromArgb(255, 250, 34, 74);
}
```
Refactor of [`RenderMesh`](https://github.com/BHoM/BHoM/blob/e0b2a43a97b9687ac0bb3d0bf3a1bc682fe879c0/Graphics_oM/Render/RenderMesh.cs#L30-L52):
```cs
public sealed class RenderMesh : Mesh, IRenderable
{
/***************************************************/
/**** Properties ****/
/***************************************************/
private List m_vertex = new List();
public override List Vertices
{
get { return m_vertex.OfType().ToList(); }
set
{
m_vertex = value.OfType().ToList();
if (value.Count != m_vertex.Count)
throw new ArgumentException($"Can only assign {nameof(RenderPoint)} to RenderMesh's {nameof(RenderMesh.Vertices)}.");
}
}
/***************************************************/
}
```
Contributor guide
Research direction
Start by reading Graphics_oM/Render/Vertex.cs and Graphics_oM/Render/RenderMesh.cs, then inspect Graphics_Engine/Convert/ToRenderMesh.cs and the related Geometry types. Review the proposed RenderPoint and RenderMesh inheritance model, along with issue #1271, to determine the scope for the other Render* objects. Done means the agreed Render* objects consistently extend their Geometry counterparts and the existing conversion duplication is addressed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- computer-graphics
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100