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

added sourceBox parameter to resizeInto in RasterYuv420Semiplanar #1852

Merged
merged 1 commit into from Aug 2, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions source/draw/RasterYuv420Semiplanar.ooc
Original file line number Diff line number Diff line change
Expand Up @@ -92,30 +92,36 @@ RasterYuv420Semiplanar: class extends RasterImage {
}
result
}
resizeInto: func (target: This) {
resizeInto: func (target: This, sourceBox := IntBox2D new()) {
if (sourceBox hasZeroArea)
sourceBox = IntBox2D new(this size)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose sourceBox := IntBox2D new(this size) does not work as a default argument?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is evaluated only once. It does not seem to be a problem now, but maybe someone would like to change the size at some point.
Should I change it anyway ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine. I think my suggestion actually does not work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:(

version(safe)
raise(sourceBox intersection(IntBox2D new(this size)) != sourceBox, "invalid source box in RasterYuv420Semiplanar resizeInto !")
thisSizeY := sourceBox height
thisSizeX := sourceBox width
thisYBuffer := this y buffer pointer
targetYBuffer := target y buffer pointer
for (row in 0 .. target size y) {
srcRow := (this size y * row) / target size y
srcRow := (thisSizeY * row) / target size y + sourceBox top
thisStride := srcRow * this y stride
targetStride := row * target y stride
for (column in 0 .. target size x) {
srcColumn := (this size x * column) / target size x
srcColumn := (thisSizeX * column) / target size x + sourceBox left
targetYBuffer[column + targetStride] = thisYBuffer[srcColumn + thisStride]
}
}
targetSizeHalf := target size / 2
thisSizeHalf := this size / 2
thisSizeHalf := sourceBox size / 2
thisUvBuffer := this uv buffer pointer as ColorUv*
targetUvBuffer := target uv buffer pointer as ColorUv*
if (target size y isOdd)
targetSizeHalf = IntVector2D new(targetSizeHalf x, targetSizeHalf y + 1)
for (row in 0 .. targetSizeHalf y) {
srcRow := (thisSizeHalf y * row) / targetSizeHalf y
srcRow := (thisSizeHalf y * row) / targetSizeHalf y + sourceBox top / 2
thisStride := srcRow * this uv stride / 2
targetStride := row * target uv stride / 2
for (column in 0 .. targetSizeHalf x) {
srcColumn := (thisSizeHalf x * column) / targetSizeHalf x
srcColumn := (thisSizeHalf x * column) / targetSizeHalf x + sourceBox left / 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I should ask: You are reading the sourceBox properties once per iteration, do you want to speed them up as you did with thisSize* and targetSize*?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lines 105, 109, 120, 124

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do that later, I'm currently still working on this function.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But thanks for noticing 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's what they pay me for.

targetUvBuffer[column + targetStride] = thisUvBuffer[srcColumn + thisStride]
}
}
Expand Down
10 changes: 10 additions & 0 deletions test/draw/RasterYuv420SemiplanarTest.ooc
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,16 @@ RasterYuv420SemiplanarTest: class extends Fixture {
(resized, source) referenceCount decrease()
output free()
})
this add("resize (with box)", func {
source := RasterYuv420Semiplanar open(this _inputPath)
box := IntBox2D new(0, source height / 2, source width, source height / 2)
target := RasterYuv420Semiplanar new(source size)
source resizeInto(target, box)
output := "test/draw/output/RasterYuv420SemiplanarTest_lowerHalfResized.png"
target save(output)
(target, source) referenceCount decrease()
output free()
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the other tests are just as "bad", but yeah, this test only calls the function, it doesn't check the output at all.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is the well-known "manual verification" or "looks good enough to me" paradigm in software testing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
free: override func {
(this _inputPath, this _inputOddWidth, this _inputOddHeight) free()
Expand Down