Bug 348942 - Graph.setBackgroundImage() does not work as expected
Summary: Graph.setBackgroundImage() does not work as expected
Status: NEW
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy Draw2d (show other bugs)
Version: 3.6.2   Edit
Hardware: PC Linux-GTK
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: gef-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-09 14:52 EDT by Mark Fenbers CLA
Modified: 2014-01-18 02:51 EST (History)
3 users (show)

See Also:


Attachments
Snippet to show background image issues (2.14 KB, application/octet-stream)
2011-06-10 19:55 EDT, Fabian Steeg CLA
no flags Details
A junit test that fails on the simple case (2.28 KB, application/octet-stream)
2012-12-08 01:55 EST, Willem Duminy CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Fenbers CLA 2011-06-09 14:52:58 EDT
graph.setBackgroundImage(myImage);
does not work properly.  The image DOES get applied, but gets overwritten immediately because some built-in mechanism is painting over it with the background color right away.  Try resizing the window (or hiding/showing it) and you should be able to see the background image for a microsecond or two before the PaintListener covers it over with the background color.  A work-around is to use
graph().getLightweightSystem().getRootFigure().setOpaque(false);
before the code given up top.
Mark
Comment 1 Mark Fenbers CLA 2011-06-10 10:02:18 EDT
The given work-around (containing setOpaque)is not without limitations.  Whenever the application launches another dialog box like a FileChooser or ColorChooser, the background image that I set in the Graph reverts back to the solid background color.
Comment 2 Fabian Steeg CLA 2011-06-10 19:55:25 EDT
Created attachment 197830 [details]
Snippet to show background image issues

It seems there are actually two separate issues here:

First, setting a background image on a graph itself has no effect. This seems to originate in FigureCanvas, as it works for Canvas, but not for FigureCanvas. The workaround does not work for me in this case (I'm on Cocoa).

Second, if I try to inherit a parent background, I see the background if I use a Canvas, but it gets covered if I use a FigureCanvas. Here I sometimes see the background briefly before it is covered. For this case, the workaround of using canvas.getLightweightSystem().getRootFigure().setOpaque(false); works for me.

I've attached a sample snippet that shows both cases.
Comment 3 Willem Duminy CLA 2012-12-08 01:55:42 EST
Created attachment 224483 [details]
A junit test that fails on the simple case

This is a proposed new test class that contains a test.  The test that fails is the one that sets the background on FigureCanvas.  I also added a test that shows the same code working for Canvas  (which should strictly speaking not be in this class).

Does the proposed test expose a bug on FigureCanvas that needs to be fixed?  Or can this behavior be explained by a design decisions previously made on FigureCanvas?
Comment 4 Alexander Nyßen CLA 2012-12-11 00:14:12 EST
(In reply to comment #3)
> Created attachment 224483 [details]
> A junit test that fails on the simple case
> 
> This is a proposed new test class that contains a test.  The test that fails
> is the one that sets the background on FigureCanvas.  I also added a test
> that shows the same code working for Canvas  (which should strictly speaking
> not be in this class).
> 
> Does the proposed test expose a bug on FigureCanvas that needs to be fixed? 
> Or can this behavior be explained by a design decisions previously made on
> FigureCanvas?

Interestingly for me (Mac OS X Mountain Lion) both test cases fail...
Comment 5 Alexander Nyßen CLA 2012-12-11 00:17:14 EST
Fabian's snippet however behaves like described, so we can use it to reproduce the issue, I suppose.
Comment 6 Alexander Nyßen CLA 2012-12-11 00:48:39 EST
Ok, I debugged into Fabian's snippet and I can exactly reproduce it on Mac OS X Mountain Lion, also that he workaround does not apply in the first case.

While I have not found out yet why the workaround does not apply in the first case documented by Fabian, it seems the issue is indeed solely caused by the fact that LightweightSytem#createRootFigure() always creates a non-opaque root figure by default. I can thus fix both of Fabian's issue by changing the initialization code within LWS#createRootFigure() to create a non-opaque root figure by default (Fabian, could you please confirm?).

However, before applying any changes to the code base, I would like to understand/clarify two more things:
1) I would like to find out why the workaround does not apply in Fabian's first reported case.
2) I would like to reproduce Mark's issue (reported in comment2) and investigate why the workaround does not apply there (probably the same issue, but I want to be sure about this).

After having clarified this, I would propose to fix the issue not by simply setting the root figure to be opaque in all cases, but to make this dependent on the SWT.BACKGROUND style being passed into the related FigureCanvas (otherwise we will break all clients that rely on the canvas to provide a solid background).
Comment 7 Alexander Nyßen CLA 2012-12-11 01:58:13 EST
I revoke my last comment. Maybe it would be misleading to use the SWT.NO_BACKGROUND or SWT.BACKGROUND style bit for this purpose.

> After having clarified this, I would propose to fix the issue not by simply
> setting the root figure to be opaque in all cases, but to make this
> dependent on the SWT.BACKGROUND style being passed into the related
> FigureCanvas (otherwise we will break all clients that rely on the canvas to
> provide a solid background).
Comment 8 Alexander Nyßen CLA 2012-12-11 02:13:45 EST
(In reply to comment #7)
> I revoke my last comment. Maybe it would be misleading to use the
> SWT.NO_BACKGROUND or SWT.BACKGROUND style bit for this purpose.
> 
> > After having clarified this, I would propose to fix the issue not by simply
> > setting the root figure to be opaque in all cases, but to make this
> > dependent on the SWT.BACKGROUND style being passed into the related
> > FigureCanvas (otherwise we will break all clients that rely on the canvas to
> > provide a solid background).

It seems I didn't mention that my fix only works if I use SWT.BACKGROUND within FigureCanvas instead of SWT_NO_BACKGROUND (which is the default option and up to now only accepted one; SWT.BACKGROUND is not accepted). So we will also have to add support for this in order to get this flying.

Maybe, introducing support for SWT.BACKGROUND and coupling the opaqueness of the LWS root figure to the SWT.BACKGROUND/SWT.NO_BACKGROUND styles would then still be the best option:
- SWT.BACKGROUND -> non-opaque root figure, so canvas background is visible
- SWT.NO_BACKGROUND -> opaque root figure, as application is responsible of filling the background
As long as SWT.NO_BACKGROUND remains to be the default option we will then not break any existing clients (who expect to have an opaque root figure), while in case of the new option (SWT.BACKGROUND), one could still change the opaqueness manually to be non-opaque in case this is required. 

Any opinions?