-
-
Notifications
You must be signed in to change notification settings - Fork 852
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
Refactor & simplify conversion API's #744
Conversation
proving that the current API is fine
keep in test code for now.
- in PixelOperations - in T4 templates - in tests
Codecov Report
@@ Coverage Diff @@
## master #744 +/- ##
==========================================
- Coverage 88.44% 88.32% -0.12%
==========================================
Files 982 983 +1
Lines 41592 41846 +254
Branches 3126 3127 +1
==========================================
+ Hits 36784 36961 +177
- Misses 4124 4203 +79
+ Partials 684 682 -2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Didn't know you could import templates like that and the bulk changes dropping the count param are welcome.
Maybe update the doc on IPixel
but I'll leave that up to your discretion.
@@ -32,7 +32,7 @@ public interface IPixel | |||
/// Sets the packed representation from a scaled <see cref="Vector4"/>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should update the docs to remove "packed"
@@ -24,93 +24,93 @@ public interface IPixel<TSelf> : IPixel, IEquatable<TSelf> | |||
} | |||
|
|||
/// <summary> | |||
/// An interface that represents a pixel type. | |||
/// A base interface for all pixels, defining the mandatory operations to be implemented by a pixel type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect!
Refactor & simplify conversion API's
Prerequisites
API cleanup
1.
IPixel
PackFrom***
==>From***
Rgba32 ToRgba32()
==>void ToRgba32(ref dest)
for perfToVector4
andToScaledVector4
were not touched: after introducing scaling to my benchmarks it turned out that the current retval form is OK2.
PixelOperations<T>
PackFrom***
->From***
count
parameter has been removed, we now followSpan.CopyTo(...)
semantics, copyingsource.Length
elements3. Bulk conversion methods on
ColorSpaceConverter
PixelOperations<T>
: The lastcount
parameter has been removed, we now followSpan.CopyTo(...)
semantics, copyingsource.Length
elementsInternal refactor
IPixel
structs and specificPixelOperations<T>
implementations were moved to a (non-namespace-provider) subfolderPixelImplementations
***.PixelOperations.Generated.tt
) by using shared code in_Common.ttinclude
MethodImpl
tricks inGuard