Bug 514847

Summary: [GTK][Win32][HiDPI] Image(Device, Image, int) constructor doesn't copy providers
Product: [Eclipse Project] Platform Reporter: Niraj Modi <niraj.modi>
Component: SWTAssignee: Niraj Modi <niraj.modi>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: arunkumar.thondapu, daniel_megert, lshanmug, ma.becker, markus.kell.r, peter, sravankumarl
Version: 4.6   
Target Milestone: 4.7 M7   
Hardware: PC   
OS: All   
See Also: https://git.eclipse.org/r/94798
https://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=e1edaaeaaf56d38ecf87fe871bfaed8ada38e2a7
Whiteboard:
Bug Depends on: 508570    
Bug Blocks: 495269, 495782    
Attachments:
Description Flags
Test snippet with sample images to reproduce this issue. none

Description Niraj Modi CLA 2017-04-06 08:01:46 EDT 
Created attachment 267677 [details]
Test snippet with sample images to reproduce this issue.

+++ This bug was initially created as a clone of Bug #508570 +++

The Image(Device, Image, int) copy constructor doesn't copy providers when the passed source image was created using below image constructors:
Image(Device, ImageDataProvider) or Image(Device, ImageFileNameProvider)

This issue was noticed while testing bug 496409 on Ubuntu, but going by the code issue would also exist on Windows(but haven't noticed this problem in real on Windows)
Comment 1 Niraj Modi CLA 2017-04-06 08:04:23 EDT 
Problem is narrowed down and I will propose a patch for M7.
Comment 2 Markus Keller CLA 2017-04-06 13:05:37 EDT 
Note that the copy constructor needs to keep copying the *current* content of the image. It can additionally copy the image providers, but these should only be used after dynamic resolution switching.

If someone modifies the image before creating a copy, then the drawing should be retained in the copy.

Example: Add this to the snippet (right after creating the images):

    cross(simple); cross(one); cross(two);

The cross(..) method:

    private static void cross(Image image) {
        Rectangle bounds= image.getBounds();
        GC gc= new GC(image);
        gc.setLineWidth(4);
        // drawLine doesn't touch alpha, see bug 457904
        gc.drawLine(0, 0, bounds.width - 1, bounds.height - 1);
        gc.drawLine(0, bounds.height - 1, bounds.width - 1, 0);
        gc.dispose();
    }
Comment 3 Eclipse Genie CLA 2017-04-11 04:41:45 EDT 
New Gerrit change created: https://git.eclipse.org/r/94798
Comment 4 Eclipse Genie CLA 2017-04-11 04:41:45 EDT 
New Gerrit change created: https://git.eclipse.org/r/94798
Comment 5 Niraj Modi CLA 2017-04-11 04:52:17 EDT 
(In reply to Markus Keller from comment #2)
> Note that the copy constructor needs to keep copying the *current* content
> of the image. It can additionally copy the image providers, but these should
> only be used after dynamic resolution switching.
> 
> If someone modifies the image before creating a copy, then the drawing
> should be retained in the copy.
> 
> Example: Add this to the snippet (right after creating the images):
> 
>     cross(simple); cross(one); cross(two);
> 
> The cross(..) method:
> 
>     private static void cross(Image image) {
>         Rectangle bounds= image.getBounds();
>         GC gc= new GC(image);
>         gc.setLineWidth(4);
>         // drawLine doesn't touch alpha, see bug 457904
>         gc.drawLine(0, 0, bounds.width - 1, bounds.height - 1);
>         gc.drawLine(0, bounds.height - 1, bounds.width - 1, 0);
>         gc.dispose();
>     }
On Windows and GTK we only maintain the content at *current* zoom(modified or not) which gets copied as part of copy constructor and we continue to do so.

With proposed patch, we now try to make a complete copy of the image *current* contents including providers and currentDeviceZoom(on GTK as well)
Comment 7 Niraj Modi CLA 2017-04-14 04:25:09 EDT 
Resolving now.