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

Extend row operation interfaces with buffer length method #2241

Merged

Conversation

ynse01
Copy link
Contributor

@ynse01 ynse01 commented Sep 26, 2022

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Some implementation of IRowOperation{T} and IRowIntervalOperation{T} interfaces use buffer length other than the width of the bounds. Examples are most convolution processors.
This PR makes the required buffer length an explicit method in these interfaces. This introduces some boiler plate code when the IRowOperation{T} is using the default, but I think the readability of the code improves even there. When re-using IRowOperation{T} implementations in other processors, the internal knowledge of the buffer length is no longer required. Last but not least, my personal trigger to start this PR: It eliminates contributors going back and forth between the RowOperation and the Processor when the buffer length is changed in the code.

Also, implemented buffered pixel conversions in Normalization processors using these extended interfaces.

I feel the existing test cover the changes in this PR already. Let me know if you feel otherwise.

@JimBobSquarePants
Copy link
Member

I agree with you here @ynse01

When I first implemented the convolution API updates that included the hack to share the buffer that was definite misuse of the existing API. This makes it much cleaner.

@JimBobSquarePants JimBobSquarePants added this to the 3.0.0 milestone Sep 27, 2022
@JimBobSquarePants JimBobSquarePants merged commit e7f701e into SixLabors:main Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants