Refactor Image(Display, int, int) in Tests #2189
Conversation
...ipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_GC.java
Show resolved
Hide resolved
...e.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_Image.java
Show resolved
Hide resolved
|
FYI, This PR is not ready to be reviewed. WIP. |
Replacing Image(Display, int, int) with Image(Display, ImageGcDrawer, int, int) in Tests
ShahzaibIbrahim
force-pushed
the
master-271
branch
from
5e92ab7 to
4aa477a
Compare
HeikoKlare
left a comment
HeikoKlare
left a comment
There was a problem hiding this comment.
Thank you for the adaptations, @ShahzaibIbrahim. Unfortunately, the bunch of changes makes it hard to properly review each and every change. There are several "regressions" in the actual changes. For example, you need to consider that the logic moved inside an ImageGcDrawer is not necessarily executed immediately but may be executed later on demand. This is particularly relevant on Windows, where an Image instantiation does not lead to a handle and/or data instantiation (anymore), such that the moved code will only be executed upon first access to an image data/handle, which may be later than expected in the subsequent code (or not at all, if no data/handle is accessed). I made comments on some of the affected placed.
Some of the manual test examples break completely. See this comparison of Bug563475_DarkDisabledTable before and after this change:
In consequence, please break down this PR into smaller changes, e.g.:
- Changes to manual tests separated from changes to automated tests, as the latter require more manual validation effort
- Simple refactorings where the image is used immediately after ImageGcDrawer definition separated from complex changes where images are passed around so that we need to be careful if the drawer is actually executed (correctly)
...lipse.swt.tests/ManualTests/org/eclipse/swt/tests/manual/Issue0146_CopyImageToClipBoard.java
Show resolved
Hide resolved
....tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_TextLayout.java
Show resolved
Hide resolved
| Image image = new Image(display, rect.width, rect.height); | ||
| gc = new GC(image); | ||
| Rectangle rect = layout.getBounds(); | ||
| final ImageGcDrawer gcDrawer = (gc, height, width) -> { |
There was a problem hiding this comment.
| final ImageGcDrawer gcDrawer = (gc, height, width) -> { | |
| final ImageGcDrawer gcDrawer = (gc, width, height) -> { |
....tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_TextLayout.java
Show resolved
Hide resolved
....tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_TextLayout.java
Show resolved
Hide resolved
....tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_graphics_TextLayout.java
Show resolved
Hide resolved
3 participants
Replacing Image(Display, int, int) with Image(Display, ImageGcDrawer, int, int) in Tests