On Fri, 16 Apr 2021 09:25:47 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
> These are 2 corner case test cases for getRenderRoot() method. > 1. `emptyDirtyRegion():` When the dirty region rect is empty i.e. (0, 0, -1, > -1) > 2. `zeroSizeDirtyRegionWithinOpaqueRegion()`: When the dirty region rect is > of zero dimensions, for example (20, 20, 0, 0) > > - emptyDirtyRegion(): When the dirty region rect is empty i.e. (0, 0, -1, -1) > See `RectBounds.makeEmpty()` and `RectBounds.isEmpty()` for definition of an > empty rect. It seems logical to NOT to perform any rendering when dirty > region is an empty rect. But current behavior is that when empty dirty region > rect is passed to `root.getRenderRoot()`, it returns the root itself in > NodePath. > The commit#1 can correct/change this behavior to return an empty NodePath. > Commit#1 is only for reference, the change is NGNode.java is a product change > which is out of scope of this fix. I shall file a new JBS issue to handle > this case. > The test `emptyDirtyRegion1` is modified and another > variant(`emptyDirtyRegion2`) of test is added. Additionally a new similar > test (`invalidDirtyRegionOutsideOpaqueRegion`) is also included which passes > an invalid rect. I think the behavior of this case should also be same as > that of empty rect. All these three tests shall be ignored using the new JBS. > > - zeroSizeDirtyRegionWithinOpaqueRegion(): When the dirty region rect is of > zero dimensions, for example (20, 20, 0, 0) > The dirty rect has 0 width and height but from the methods > `RectBounds.makeEmpty()` and `RectBounds.isEmpty()` we can infer that the > rect(20, 20, 0, 0) is not an empty rect. (It may be considered as a dirty > point). So if we consider this rect as a valid dirty region then the current > behavior seems valid. I have added a couple more similar tests. Yes, filing a new bug to evaluate case 1 seems like the best option. We might decide that is a product bug or we might end up deciding that the behavior is correct, in which case it would be a test bug (it seems at least possible that an empty bounds is used to mean "render the whole window", and if so, we would not want to make a change since it would cause a regression). ------------- PR: https://git.openjdk.java.net/jfx/pull/463