AcademySoftwareFoundation / AcademySoftwareFoundation/Imath

Nonsensical Matrix22::multDirMatrix

Open
#62 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
488
Forks
161
Avg merge
1d 3h
Merged PRs (30d)
3

Description

Matrix44 methods multDirMatrix and multVecMatrix operate on Vec3, doing the math as if it were Vec4 with the last component of 0 and 1, respectively, and then projecting back to 3D with a homogeneous divide. (For those who don't know, this is what allows 4x4 matrices to transform 3D points and vectors and capture effects such as translation and perspective projection.) Matrix33 has the same methods, operating on Vec2, performing 2D transformations with homogeneous 3x3 matrix.

Matrix22 also has multDirMatrix... but it operates on a Vec2 and has no homogeneous divide. So the nomenclature is incorrect, this is not a "direction" kind of multiply. It's just a regular mathematical 2x2 matrix transforming a 2D point (i.e., a synonym for the `operator*(Vec2,Mat22)`), not the equivalent operation of the Matrix33 and 44 methods having that name.

Does this bother anyone else? Should we maybe just remove this Matrix22 method?

Contributor guide

Open the contributing guide

Research direction

Start by inspecting Matrix22::multDirMatrix and compare its behavior and naming with the Matrix33 and Matrix44 methods described in the issue. Check the related Vec2 and matrix multiplication APIs to determine whether removal or a compatibility-preserving rename is appropriate. Done means the API decision is made and the affected behavior and tests are updated consistently.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
computer-graphics
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.