Bug 514847 - [GTK][Win32][HiDPI] Image(Device, Image, int) constructor doesn't copy providers
Summary: [GTK][Win32][HiDPI] Image(Device, Image, int) constructor doesn't copy providers
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.6   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 4.7 M7   Edit
Assignee: Niraj Modi CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 508570
Blocks: 495269 495782
  Show dependency tree
 
Reported: 2017-04-06 08:01 EDT by Niraj Modi CLA
Modified: 2017-05-22 05:16 EDT (History)
7 users (show)

See Also:


Attachments
Test snippet with sample images to reproduce this issue. (5.89 KB, application/zip)
2017-04-06 08:01 EDT, Niraj Modi CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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.