Bug 526290 - Control#drawsBackground should evaluate SWT.NO_BACKGROUND
Summary: Control#drawsBackground should evaluate SWT.NO_BACKGROUND
Status: RESOLVED INVALID
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.8   Edit
Hardware: PC Mac OS X
: P3 normal (vote)
Target Milestone: 4.9   Edit
Assignee: Karsten Thoms CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
: 238918 (view as bug list)
Depends on:
Blocks: 238918 366471
  Show dependency tree
 
Reported: 2017-10-20 03:19 EDT by Karsten Thoms CLA
Modified: 2018-06-18 07:05 EDT (History)
8 users (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Karsten Thoms CLA 2017-10-20 03:19:46 EDT 
The default implementation just returns true, but it should return false when style SWT.NO_BACKGROUND ist specified.

This would resolve bug#238918, but also fix other controls. They call fillBackground(), which does a fast return when the NO_BACKGROUND style is applied.
Comment 1 Eclipse Genie CLA 2017-10-20 03:22:48 EDT 
New Gerrit change created: https://git.eclipse.org/r/110421
Comment 2 Karsten Thoms CLA 2017-11-27 14:53:05 EST 
*** Bug 238918 has been marked as a duplicate of this bug. ***
Comment 3 Alexander Kurtakov CLA 2017-11-30 04:29:44 EST 
Lakshmi would you please review this one?
Comment 4 Lakshmi P Shanmugam CLA 2017-12-04 05:51:02 EST 
Hi Karsten,
In Cocoa code, Composite.drawBackground() already checks if SWT.NO_BACKGROUND is set and calls fillBackground() only if its not set.
I didn't find any Control that has the SWT.NO_BACKGROUND style set. So, why is the check in Control.drawsBackground() required? Am I missing something?
Comment 5 Karsten Thoms CLA 2017-12-04 07:00:57 EST 
I think I came to this as follows:
- bug#238918 requests to evaluate SWT.NO_BACKGROUND for Toolbar
- the attached patch wanted to evaluate this in Toolbar#drawBackground
- that method calls Control#fillBackground
- the first check there is calling drawsBackground and does a fast return if that evaluates to false
=> the better place to evaluate SWT.NO_BACKGROUND is in drawsBackground

I think the problem is here that we cannot tell why the reporter wanted to evaluate it for a Toolbar, maybe this happens in custom code?

Or asking the other way around: What is speaking against users setting this style to controls?
Comment 6 Lakshmi P Shanmugam CLA 2017-12-05 08:06:55 EST 
(In reply to Karsten Thoms from comment #5)
> I think the problem is here that we cannot tell why the reporter wanted to
> evaluate it for a Toolbar, maybe this happens in custom code?
> 
Toolbar extends Composite, so NO_BACKGROUND will be checked in Composite.drawsBackground(). I'm not sure why he was evaluating the flag, the patch was for Carbon code, so can't say without looking at Carbon code.

> Or asking the other way around: What is speaking against users setting this
> style to controls?
I think for native controls, the background is drawn by Cocoa and if we prevent the drawing, the correct background will not be drawn. If this is true, we cannot allow a flag in the client code to decide if the Control should/shouldn't draw the background.
Comment 7 Lakshmi P Shanmugam CLA 2017-12-05 08:10:00 EST 
Moving bug out of M4 to 4.8 for now.
Comment 8 Niraj Modi CLA 2018-05-14 05:55:32 EDT 
Moving to 4.9, please re-target as required.
Comment 9 Lakshmi P Shanmugam CLA 2018-06-18 07:05:18 EDT 
I believe there is nothing to be done here since SWT.NO_BACKGROUND should not be set for a Control (please see comment #6).