WIP: PlaneRegion extension, deployment, buffer work, and fix for #2360 #2390
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xiphmont
force-pushed
the
work_on_2360_b
branch
2 times, most recently
from
5f82f9a
to
a1af628
Compare
src/tiling/plane_region.rs
Outdated
| // This will _not_ allow expanding a region beyond the original | ||
| // rectangle created with new() or from_slice(). As such, it | ||
| // protects the original Tile/Frame boundaries. | ||
| pub fn superregion_mut(&self, area: Area) -> PlaneRegionMut<'_, T> { |
src/tiling/plane_region.rs
Outdated
| @@ -115,6 +144,7 @@ pub struct PlaneRegion<'a, T: Pixel> { | |||
| pub plane_cfg: &'a PlaneConfig, | |||
| // private to guarantee borrowing rules | |||
| rect: Rect, | |||
| max_rect: Rect, | |||
There was a problem hiding this comment.
I already have concerns about the amount of data being passed around with PlaneRegions. You should create a separate struct for when you actually need this data.
There was a problem hiding this comment.
I'm not at all sure what the concern is... could you elaborate? the 16 bytes for a rect saves a few large plane copies and provides an addiitonal measure of limits checking as now PlaneRegions can be used where before it's needed to copy into a frame or use slices (that can only enforce beginning/end).
The only decent way I see to keep this simple is to extend the abstraction transparently. The goal here is to make mistakes harder.
There was a problem hiding this comment.
Oh! Or do you mean 'PlaneRegion wasn't meant for this' and you're concerned it's being--- perverted....
…cision pipeline Patch returns most of the LF RDO pipeline to Pixel<T> rather than hardwiring to Pixel<u16>. The intent is to re-allow use of lbd assembly for cdef, LRF, etc. More work needs to be done to eliminate redundant buffers, copies, etc, but this gets most of the typing work done for testing.
...at the cost of more Generics combinations in the deblocking code. The patch expands the generic typing so deblocking can take mismatched input depths on reference and reconstruction (internal math is all i32 anyway).
…ions Extend PlaneRegions to allow wrapped re-expansion of subregions via superregion, superregion_mut and SuperIndex. Expansion is only allowed out to the boundaries of the original PlaneRegion when it was created via new() or from_slice() (and, by extension, from the tiler as well as region(), as_region(), etc). In short, the original tile boundaries are still enforced during resizing and dereferencing. superregion() and superregion_mut() are simply versions of subregion() and subregion_mut() that allow a negative x,y offset, as well as a size up to the original rectangle size. SuperIndex is simply an isize such that Index<SuperIndex> can dereference outside the current declared y/height, but not beyond the original Frame/Tile bounds. The patch then continues by replacing use of PlaneSlice with PlaneRegion throughout the loopfilters and loopfilter RDO, where use was previously split between the two (because PlaneSlice offers the entire place ~ without any real bounds checking).
Sanitized, 2-D access to the visible or padded region of a PlaneRegion. Currently implemented through index operator overloading via some horrifying fat pointer abuse. "I don't think slices are meant to do that." "Yeah, but they're doing it!"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch set is a multi-pronged approach at improving buffer usage in the loop filters by using a single consistent region interface (PlaneRegion), extending the PlaneRegion to add functionality previously only available from PlaneSlice while offering better boundary checking, eliminating a couple unneded copies/temp planes, and restoring the loop filter RDO loop to (mostly) Pixel such that 8 bit assembly comes back into play along with reduced memory bandwidth pressure.
Ref: #2360