Created method for slicing ROI into tiles, for use inside AsyncEffectRenderer
#983
Conversation
…gs`, also `OtherExtensions.VerticalSliceup` was created
| { } | ||
| } | ||
|
|
||
| public static IEnumerable<RectangleI> VerticalSliceup (this RectangleI original) |
There was a problem hiding this comment.
The naming is a bit confusing to me, since it makes me think of slicing into vertical columns.
Perhaps something like ToRows() or GenerateRows() etc?
This could also just be a method on RectangleI similar to other methods like Inflate - not sure this really needs to be an extension
There was a problem hiding this comment.
It is now named ToRows
| if (tileWidth < 0) throw new ArgumentOutOfRangeException (nameof (tileWidth), "Cannot be negative"); | ||
| if (tileHeight < 0) throw new ArgumentOutOfRangeException (nameof (tileHeight), "Cannot be negative"); | ||
| if (renderBounds.Width < 0) throw new ArgumentException ("Width cannot be negative", nameof (renderBounds)); | ||
| if (renderBounds.Height < 0) throw new ArgumentException ("Cannot be negative", nameof (renderBounds)); |
There was a problem hiding this comment.
The message should say Height cannot be negative
| TileWidth = tileWidth; | ||
| TileHeight = tileHeight; | ||
|
|
||
| // If the effect isn't tileable, there is a single tile for the entire render bounds. |
There was a problem hiding this comment.
This comment doesn't really make sense here - maybe it should be lower down around the call to VerticalSliceup()?
|
|
||
| try { | ||
|
|
||
| bounds = GetTileBounds (tileIndex); | ||
| bounds = target_tiles[tileIndex]; // Get tile bounds |
There was a problem hiding this comment.
This doesn't really need to be in the try/catch block anymore
|
Thank you for the feedback @cameronwhite, I have addressed your points |
This should get us a step closer towards handling parallelism inside the effects themselves, as discussed in #808