Replace usages of new Image(device, width, height) #2178
Conversation
Fixing it with test intact.
There was a problem hiding this comment.
Some comments on the coding style and unnecessary changes.
@akoch-yatta can you also please take a look? This PR solves the issue by instantiating gc again in line 1602 .
|
|
||
| if ((style & (SWT.NO_BACKGROUND | SWT.TRANSPARENT)) != 0) { | ||
| /* This code is intentionally commented because it may be slow to copy bits from the screen */ | ||
| //newPaintGC.copyArea (image, ps.left, ps.top); | ||
| } else { | ||
| RECT rect = new RECT (); | ||
| OS.SetRect (rect, ps.left, ps.top, ps.right, ps.bottom); | ||
| drawBackground (gc.handle, rect); | ||
| } | ||
|
|
There was a problem hiding this comment.
Why duplicate this code?
| if ((style & (SWT.NO_BACKGROUND | SWT.TRANSPARENT)) != 0) { | |
| /* This code is intentionally commented because it may be slow to copy bits from the screen */ | |
| //newPaintGC.copyArea (image, ps.left, ps.top); | |
| } else { | |
| RECT rect = new RECT (); | |
| OS.SetRect (rect, ps.left, ps.top, ps.right, ps.bottom); | |
| drawBackground (gc.handle, rect); | |
| } |
| ImageGcDrawer drawer = new ImageGcDrawer() { | ||
| @Override | ||
| public void drawOn(GC gc, int iWidth, int iHeight) { | ||
| GCData gcData = gc.getGCData (); |
There was a problem hiding this comment.
The indentation looks wrong in Eclipse (not in GH though)
| if (!isDisposed ()) { | ||
| paintGC.drawImage (image, DPIUtil.scaleDown(ps.left, zoom), DPIUtil.scaleDown(ps.top, zoom)); | ||
| } | ||
| gc.dispose(); |
There was a problem hiding this comment.
Revert this change, it's unnecessary now that you instantiated gc again and it only adds noise to the PR.
| } | ||
| }; | ||
| image = new Image (display, drawer, width, height); | ||
| gc = new GC(image, originalStyle & SWT.RIGHT_TO_LEFT); |
There was a problem hiding this comment.
Are you sure that this works as expected? You now initialize the GC inside the ImageGcDrawer and then create a new one that is used throughout the rest of the method, which does not adhere to the settings inside ImageGcDrawer. At least, it would be my expectation that those two GCs behave completely independent. In my understanding, the code you move inside the ImageGcDrawer now equals to a no-op, doesn't it?
The original commit was reverted in #2163 due to regression. With the test added, this PR again replaces the usage of Image(device, width, height) with Image(device, ImageGcDrawer).