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

[API Proposal]: Matrix4x4.CreateViewport #79961

Closed
PavielKraskouski opened this issue Dec 25, 2022 · 6 comments · Fixed by #87748
Closed

[API Proposal]: Matrix4x4.CreateViewport #79961

PavielKraskouski opened this issue Dec 25, 2022 · 6 comments · Fixed by #87748
Labels
api-approved API was approved in API review, it can be implemented area-System.Numerics
Milestone

Comments

@PavielKraskouski
Copy link

PavielKraskouski commented Dec 25, 2022

Background and motivation

Matrix4x4 currently provides a large number of methods for creating various transformation matrices. The figure below shows a geometric pipeline for transforming vertices from object coordinates to window coordinates. Unfortunately, Matrix4x4 does not provide a method for creating a matrix that transforms vertices from normalized device coordinates to window coordinates.

image

I propose to add a method for creating a viewport matrix, which is shown in the figure below. width and height specify viewport dimensions, x and y stand for coordinates of the upper left corner of the viewport, zmin and zmax define the minimum and maximum viewport depth.

image

API Proposal

namespace System.Numerics;

public struct Matrix4x4
{
    public static Matrix4x4 CreateViewport(float x, float y, float width, float height, float minDepth, float maxDepth)
    {
        Matrix4x4 result = Identity;
        
        result.M11 = width * 0.5f;
        result.M22 = -height * 0.5f;
        result.M33 = maxDepth - minDepth;
        result.M41 = x + result.M11;
        result.M42 = y - result.M21;
        result.M43 = minDepth;

        return result;
    }
}

API Usage

Matrix4x4 scaleMatrix = Matrix4x4.CreateScale(model.Scale);
Matrix4x4 rotationMatrix = Matrix4x4.CreateFromYawPitchRoll(model.Yaw, model.Pitch, model.Roll);
Matrix4x4 translationMatrix = Matrix4x4.CreateTranslation(model.Position);
Matrix4x4 modelMatrix = scaleMatrix * rotationMatrix * translationMatrix;
Matrix4x4 viewMatrix = Matrix4x4.CreateLookAt(camera.Position, camera.Target, camera.Up);
Matrix4x4 projectionMatrix = Matrix4x4.CreatePerspectiveFieldOfView(camera.Fov, camera.AspectRatio, camera.Near, camera.Far);
Matrix4x4 modelViewProjectionMatrix = modelMatrix * viewMatrix * projectionMatrix;
Matrix4x4 viewportMatrix = Matrix4x4.CreateViewport(0, 0, bitmap.PixelWidth, bitmap.PixelHeight, 0, 1);

Vector4[] windowVertices = new Vector4[model.Vertices.Length];
for (int i = 0; i < windowVertices.Length; i++)
{
    windowVertices[i] = Vector4.Transform(model.Vertices[i], modelViewProjectionMatrix);
    windowVertices[i] /= windowVertices[i].W;
    windowVertices[i] = Vector4.Transform(windowVertices[i], viewportMatrix);
}

DrawPoints(windowVertices);

Alternative Designs

No response

Risks

No response

@PavielKraskouski PavielKraskouski added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Dec 25, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Dec 25, 2022
@ghost
Copy link

ghost commented Dec 25, 2022

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

The Matrix4x4 class currently provides a large number of methods for creating various transformation matrices. The figure below shows a geometric pipeline for transforming vertices from object coordinates to window coordinates. Unfortunately, the Matrix4x4 class does not provide a method for creating a matrix that transforms vertices from normalized device coordinates to window coordinates.

image

I propose to add a method for creating a viewport matrix, which is shown in the figure below. width and height specify viewport dimensions, x and y stand for coordinates of the upper left corner of the viewport.

image

API Proposal

namespace System.Numerics;

public struct Matrix4x4
{
    public static Matrix4x4 CreateViewport(float x, float y, float width, float height)
    {
        Matrix4x4 result = Identity;
        
        result.M11 = width * 0.5f;
        result.M21 = -height * 0.5f;
        result.M41 = x + result.M11;
        result.M42 = y - result.M21;

        return result;
    }
}

API Usage

Matrix4x4 scaleMatrix = Matrix4x4.CreateScale(model.Scale);
Matrix4x4 rotationMatrix = Matrix4x4.CreateFromYawPitchRoll(model.Yaw, model.Pitch, model.Roll);
Matrix4x4 translationMatrix = Matrix4x4.CreateTranslation(model.Position);
Matrix4x4 modelMatrix = scaleMatrix * rotationMatrix * translationMatrix;
Matrix4x4 viewMatrix = Matrix4x4.CreateLookAt(camera.Position, camera.Target, camera.Up);
Matrix4x4 projectionMatrix = Matrix4x4.CreatePerspectiveFieldOfView(camera.Fov, camera.AspectRatio, camera.Near, camera.Far);
Matrix4x4 modelViewProjectionMatrix = modelMatrix * viewMatrix * projectionMatrix;
Matrix4x4 viewportMatrix = Matrix4x4.CreateViewport(0, 0, bitmap.PixelWidth, bitmap.PixelHeight);

Vector4[] windowVertices = new Vector4[model.Vertices.Length];
for (int i = 0; i < windowVertices.Length; i++)
{
    windowVertices[i] = Vector4.Transform(model.Vertices[i], modelViewProjectionMatrix);
    windowVertices[i] /= windowVertices[i].W;
    windowVertices[i] = Vector4.Transform(windowVertices[i], viewportMatrix);
}

DrawPoints(windowVertices);

Alternative Designs

No response

Risks

No response

Author: PavielKraskouski
Assignees: -
Labels:

api-suggestion, area-System.Numerics, untriaged

Milestone: -

@tannergooding
Copy link
Member

Viewports are typically represented as distinct types and not directly as matrices. They likewise typically include a min/max depth in their representation and are not exclusively location + size.

I'd expect a more typical matrix created from a viewport would be (noting this matches yours for the "typical" minZ/maxZ values of 0 and 1, represetively):

w / 2          0              0              0
0             -h / 2          0              0
0              0              maxZ - minZ    0
x + (w / 2)    y + (h / 2)    minZ           1

That being said, exposing a "proper" viewport type is probably more than desirable and exposing some API that creates the corresponding matrix from the relevant 6 parameters is probably fine.

There are potentially some open issues/questions here, however. In particular, this is just one example of a valid "viewport" matrix and is one that (iirc) places 0, 0 at the upper left corner (y increases downwards). However, other setups may allow for a 0, 0 that starts in the lower left corner/etc (sometimes achieved by allowing negative heights, etc).

Much of that can be answered in the docs, but we may want to consider whether just "CreateViewport" is the correct name and what parameter validation, if any, should be done; etc.

@tannergooding tannergooding added needs-author-action An issue or pull request that requires more info or actions from the author. and removed untriaged New issue has not been triaged by the area owner labels Jan 3, 2023
@ghost
Copy link

ghost commented Jan 3, 2023

This issue has been marked needs-author-action and may be missing some important information.

@tannergooding
Copy link
Member

If you could update the API proposal to include the minDepth/maxDepth then I think we can move forward with the proposal.

@PavielKraskouski
Copy link
Author

@tannergooding Updated.

@ghost ghost added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed needs-author-action An issue or pull request that requires more info or actions from the author. labels Jan 3, 2023
@tannergooding tannergooding added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration labels Jan 3, 2023
@tannergooding tannergooding added this to the 8.0.0 milestone Jan 3, 2023
@bartonjs
Copy link
Member

bartonjs commented Jan 17, 2023

Video

Looks good as proposed

namespace System.Numerics;

public partial struct Matrix4x4
{
    public static Matrix4x4 CreateViewport(float x, float y, float width, float height, float minDepth, float maxDepth);
}

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jan 17, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 18, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 23, 2023
@dotnet dotnet locked as resolved and limited conversation to collaborators Jul 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Numerics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants