Community
Participate
Working Groups
Windows Vista and 7 allow shell's to have transparency known as Glass. SWT should support this feature so that applications can take advantage of this to make modern looking UI's. Widgets can be placed on the glass area so that they look integrated with the application title bar. For examples, see the tab bar of IE 9 or Chrome.
Created attachment 179279 [details] Screeshot An example of a
The attached image is an example of a combo box placed on the Glass area in IE 8.
Created attachment 181753 [details] Initial patch with basic support for Glass on Shell
I believe Silenio and Bogdan look at this problem again not too long ago. The main problems where (as far as I know): 1) It is a Window only concept, super hard to design a platform independent API providing this functionallity. 2) Increasing the aero glass takes space from the client area of the shell, this breaks all existent code that relies in the current origin of the client area. Personally I think bug 329292, bug 329293, and bug 329299 are not useful and should be closed.
Felipe, Regarding second problem, this is optional feature that developers can use to enchance look&feel of their applications on Windows Vista, 7 and upward, nothing will be imposed on currently designed and developed windows/dialogs. As for the first problem I completely agree that designing API will be very hard, that's why I've submited early version of the patch to have discussion started. Submitted bugs are relevant only to widgets that are hard to integrate with glass pane, Text widgets has a problem with WM_PAINT and draw without buffering, and Label needs to be have paint method reimplemented. I think they are key parts of the Glass patch for SWT and needs to be treated this way.
Felipe, Silenio, Is there anything that we can do to move forward with this as it would be a great addition for RCP developers and could enable some interesting things for e4. There are other Platform dependent concepts (i.e. sheets from MAc) that are a part of SWT and help improve the experience on Eclipse and other RCP applications for users.
Created attachment 185193 [details] Patch with minimal changes to API I'm attaching different attempt at bringing Aero Glass into SWT. This time we don't specify windows centric methods, instead we use internal widget's data. Overriden method setData triggers update to glass when specific data key is provided. Additionaly DwmChange event is being sent when change to system theme occurs. In my opinion this approach is less invasive than previous one.
(In reply to comment #7) > In my opinion this approach is less invasive than previous one. As far the code goes it seems to have some problems but not bad. Control#isGlass() apparently is never called. since Control#isGlass() is never called, GLASS_PANE is useless. I see a remove icon and caption, but I don't see code to put it back when GLASS_FLAGS changes. etc (sorry if I came to any wrong conclusion, but I didn't run the code, just read it) As far the API goes, you have Display#getSystemTheme() - that is okay I think Then you have: public static final int DwmChange = 102; public static final int WINDOWS_THEME_CLASSIC = 0; public static final int WINDOWS_THEME_DEFAULT = 1; public static final int WINDOWS_THEME_GLASS = 2; public static final int GLASS_NOICON = 1; public static final String GLASS_AREA = "GlassArea"; //$NON-NLS-1$ public static final String GLASS_FLAGS = "GlassFlags"; //$NON-NLS-1$ public static final String GLASS_PANE = "GlassPane"; //$NON-NLS-1$ That is windows-centric API. Agreed ? As far the idea in general, not sure how people will use it as it does not solve the problem with origin of the client area. when the GLASS_AREA is set it is up to the application code to handle the problem. For example, if the shell has any layout manager it won't work (it will place controls over the glass etc). Correct ?
Hi Felipe, Control#isGlass() is actually used in other widgets to determine if widget is placed on glass part of the Shell. I didn't provide patches for widgets as I'm working on them (trying to iron out some bugs). As for WINDOWS_THEME_* constants I've completely forgot to rename them (will do that with next patch) to remove WINDOWS_ part. Yes, developers would have to consciously use the feature which means they need to provide a Composite widget that will have GLASS_PANE data set. There's no easy way to provide out-of-the-box easy to use solution to this kind of feature.
Felipe, Silenio, any thought on getting this support into 3.7?
Is there anything Shawn, Lukasz or I could do on our end to help move this forward?
I understand if the Platform SWT team is busy with other plan items, and want to make sure that we can move this forward with the absolute minimal amount of effort on your side. Along with or counterparts at Microsoft we see this as an important part of Eclipse SWT modernization for Windows 7. Is there anything that we can do to help move it forward in time for Indigo?
Hi Mik, I talked to Bogdan (Silenio is away on vacation), here is our position: We would like to offer this support for Windows and Cocoa if we can provide a common API that can sit on top of both platforms. Adding an API to SWT that mimics the Windows API (likely mapping method/constants one-to-one) is not what we want as it won't work when implemented on Cocoa. Windows, you know how it works, you can increase the size of the trim in to client area. The application has to handle layout taking in consideration that the origin of the client area is in the wrong place (for the controls that are to be placed in the trim) . Cocoa, on the other hand, returns a Toolbar object that can be used to place additional controls. The client area remains the same way (thus all the layout code can remains unchanged). When we tried to write a common API we chose the follow the Cocoa model, so we had to implement it on top of the support windows offers. Following this path we would add a method to shell to return either a ToolBar or a Composite to be used to place other controls. (I don't think we can have a Composite in Cocoa, so probably it would have to be a Toolbar). Note that with a Toolbar it is still possible to place any arbitrary control by using a separator ToolItem and the setControl API. (See Shell#getToolBar). I don't think is possible to mimic the windows API on cocoa. This leaves us with two implementation options for win32: a) Using multiple handles in Shell, at least one top handle for the entire shell, and one handle for the client area. You would have to visit all the places and make sure the correct handle is being used for a given operation: setBounds for example has to done for the top handle, adding children and mouse events have to be done for client area handle. Getting this right is a lot of work (basically a rewrite Shell). b) Keep one handle for the Shell, but "lie" for the y in several calls (mouse event coordinates, getBounds/setBounds for children, getClientArea, etc). Again, this is a lot of work. You will need to be very thorough to make sure you don't miss any place.
(In reply to comment #13) > Windows, you know how it works, you can increase the size of the trim in to > client area. The application has to handle layout taking in consideration that > the origin of the client area is in the wrong place (for the controls that are > to be placed in the trim) . > > When we tried to write a common API we chose the follow the Cocoa model, so we > had to implement it on top of the support windows offers. > Following this path we would add a method to shell to return either a ToolBar > or a Composite to be used to place other controls. > (I don't think we can have a Composite in Cocoa, so probably it would have to > be a Toolbar). Note that with a Toolbar it is still possible to place any > arbitrary control by using a separator ToolItem and the setControl API. (See > Shell#getToolBar). I don't think is possible to mimic the windows API on cocoa. Actually, I think it is possible to do this on Cocoa, but there's no official API to do it. The superview of the view returned by NSWindow#contentView is the view that holds the controls and toolbar area of a standard document window. Google Chrome does this -- that's how they are able to draw their tabs in the title area of the window. For Aero, does this mean that the window's origin is still at the top-left of the entire Shell or is the content/cleint area effectively moved down? If it's the latter, I think this would make it possible to have one API. You could return a Composite for the 'trim area', which is still parented to the Shell, and then the controls in the content area of the window are also parented to the Shell. The question of 'where is the origin' is probably the biggest one in my mind, but I would think that for compatibility, this trim area Composite would have negative coordinates, relative to the current origin, which is at the top left of the content area. That way, if some plugin comes along and wants to put something in this trim area, any existing code would continue to work without breaking any assumptions. You wouldn't be able to do both 'getTrimContainer' or whatever it's called and 'getToolbar', since the NSToolbar on Cocoa uses this area but I think that's fine. I'm probably leaving something out but it's a starting point.
Reading the last couple of comments, this sounds like it's a non-trivial amount of work and it could be destabilizing (i.e. bugs in this code could impact anybody using a Shell). I'm also worried that the only way to do this is on the Mac appears to be via reaching into internals. How confident are we that that won't simply be busted by the next version of OS X? In terms of *usability*, I'd like to understand what we expect applications to do when they are run on platforms that do not support the capability. Ideally, it would be great if they didn't have to write platform specific code, but even if they did, we should be able to provide guidelines/best practices around it. Can someone produce mockups, maybe with a more interesting set of controls than the first screenshot, that show what we expect it to look like on Win7, Mac and Linux (and/or Win XP)? Overall, I'm worried about this.
(In reply to comment #15) > Reading the last couple of comments, this sounds like it's a non-trivial amount > of work and it could be destabilizing (i.e. bugs in this code could impact > anybody using a Shell). I'm also worried that the only way to do this is on > the Mac appears to be via reaching into internals. How confident are we that > that won't simply be busted by the next version of OS X? Implementation-wise, on Cocoa it's not all that hard to do, but I do think there's a lot of SWT API that assumes the Shell is itself the top-most container in the hierarchy. This would be violating that assumption. I am pretty confident, though, that a future OS wouldn't break it, because you are using API to get at the title bar area. Safari and Google Chrome are already doing this and I'd be very surprised if Apple did something that would break either of those apps. > Can someone produce mockups, maybe with a more interesting set of controls than > the first screenshot, that show what we expect it to look like on Win7, Mac and > Linux (and/or Win XP)? As mentioned in comment #1, IE 9 and Google Chrome do this on Windows 7. Don't know about linux.
(In reply to comment #15) > In terms of *usability*, I'd like to understand what we expect applications to > do when they are run on platforms that do not support the capability. Ideally, > it would be great if they didn't have to write platform specific code, but even > if they did, we should be able to provide guidelines/best practices around it. > Can someone produce mockups, maybe with a more interesting set of controls than > the first screenshot, that show what we expect it to look like on Win7, Mac and > Linux (and/or Win XP)? McQ: My view on this is that from the higher level and usability points of view, we follow what modern apps such as Microsoft Office and Google Chrome are doing right now. If glass is supported (Windows 7), it's rendered. If it's not supported (Windows XP, Windows 7 without Aero, Mac, Linux) the existing widget color or shading is used. If not all controls can be put on the glass area, the application (Eclipse, RCP-based or otherwise) can decide not to put them there. What I think is a win for Platform/SWT for Eclipse 3.7 is providing this ability in a way that can be leveraged with at least some end-user friendly functionality on Windows (hopefully Mac as well) as we did for 3.6 (http://download.eclipse.org/eclipse/downloads/drops/R-3.6-201006080911/eclipse-news-part3.html#SWT) and with the ability to improve on that in future releases. Given that one of the cleanest and also most sophisticated uses of that is Google Chrome, I will post a cross-platform screenshot of that.
Created attachment 188488 [details] screenshot of Google Chrome on Windows 7 with Aero and on Mac OS X
Thanks for the screenshots Mik. Believe me, I get that we want to be able to build "modern" looking apps with SWT. At some level though, the places where we *can* support this are the uninteresting cases. For this to make sense, I believe we need to understand two things: 1) What does the code look like to support the two different presentations? If it's just a matter of sticking the controls in a different composite and setting some flags on the shell (or such like), then it's fine with me. What we don't want is for it to require pervasive changes to the app structure, so that once you've decided to write your app for Win7 it becomes a "big deal" to switch it to run on Linux. You see? 2) In terms of the _look_and_feel_ of apps that get built, what do we expect to see end up in the titlebar area and what do we think the equivalent presentation would be when you can't put them up there? I realize that "the application (Eclipse, RCP-based or otherwise) can decide not to put them there" is fundamentally true, but if the result looks like crap everywhere but Win7, it's going to suck. Anyway, can we set up a call about this? I think Felipe's suggestion in comment #8 is on the right track, but I haven't thought about it enough... (And if we are trying to get this in for 3.7, then we need to move faster than bugzilla.)
Thanks McQ, that sounds good, figure out how to address those points on a call. I'll get it scheduled on this end with you and then post the conf call details for anyone else who wants to join. To seed the discussion, I totally agree with your view on (1). Regarding (2), my thinking is that the typical Eclipse shell, including the Workbench, would use the same layout with and without glass. Similar to how Office 2010 does, ie, no layout changes, just added transparency for the menu bar and trim-located toolbar when Aero is on. But also enable RCP app developers would have the option of getting fancier with glass backgrounds in other widgets, eg, using it to make native looking trims and toolbar widgets on Windows 7 while they opt for the native Mac toolbar (bug 222859) on OS X.
(In reply to comment #14) > (In reply to comment #13) > > Windows, you know how it works, you can increase the size of the trim in to > > client area. The application has to handle layout taking in consideration that > > the origin of the client area is in the wrong place (for the controls that are > > to be placed in the trim) . > > > > When we tried to write a common API we chose the follow the Cocoa model, so we > > had to implement it on top of the support windows offers. > > Following this path we would add a method to shell to return either a ToolBar > > or a Composite to be used to place other controls. > > (I don't think we can have a Composite in Cocoa, so probably it would have to > > be a Toolbar). Note that with a Toolbar it is still possible to place any > > arbitrary control by using a separator ToolItem and the setControl API. (See > > Shell#getToolBar). I don't think is possible to mimic the windows API on cocoa. > > Actually, I think it is possible to do this on Cocoa, but there's no official > API to do it. The superview of the view returned by NSWindow#contentView is the > view that holds the controls and toolbar area of a standard document window. > > Google Chrome does this -- that's how they are able to draw their tabs in the > title area of the window. > > For Aero, does this mean that the window's origin is still at the top-left of > the entire Shell or is the content/cleint area effectively moved down? If it's > the latter, I think this would make it possible to have one API. You could > return a Composite for the 'trim area', which is still parented to the Shell, > and then the controls in the content area of the window are also parented to > the Shell. You can modify client area by responding to WM_NCCALCSIZE, but if you will not do it, client area is not changed. Google Chrome to draw tabs inside trim area responds to WM_NCCALCSIZE. > The question of 'where is the origin' is probably the biggest one in my mind, > but I would think that for compatibility, this trim area Composite would have > negative coordinates, relative to the current origin, which is at the top left > of the content area. That way, if some plugin comes along and wants to put > something in this trim area, any existing code would continue to work without > breaking any assumptions. > > You wouldn't be able to do both 'getTrimContainer' or whatever it's called and > 'getToolbar', since the NSToolbar on Cocoa uses this area but I think that's > fine. > > I'm probably leaving something out but it's a starting point. I'm a bit worried that everyone is talking about only one container. You can extend trim area not only on top side of the window, but actually make the trim grow inside the window, good example would be Windows Media Player. I'm aware that making top of the window glass enabled would be 90% of cases, but we need also to look at those 3 other positions.
Felipe, Making all of the comments above into consideration, could you advise in which direction I should be preparing this feature?
WM_NCCALCSIZE will change the client area, but if you use it to move the client area down a bit (so as to have the glass at the top) you are left with a big hole in the shell. There's probably a way around this issue but we're trying a completely different approach to this API, which features minimal code change and allows you to place controls wherever you want (not just the top). We hope to be able to attach some code later on today.
Created attachment 189281 [details] PI hacks I'm adding this PI Hack in case someone wants to try it, it very simple and helps understand the windows API.
Created attachment 189283 [details] Testcase for the bit approach This a test case showing a new approach for this problem.
Created attachment 189284 [details] Testcase running on Windows 7
Created attachment 189286 [details] Testcase running on Mac
Created attachment 189287 [details] Patch for the Mac
Created attachment 189288 [details] Patch for Windows
The idea it to add a style bit (SWT.TRIM_FILL) When a Shell is created with it and entire Shell is trim (glass on windows, windows texture on mac). When a Composite is created with it, the composite background is trim (basically makes the composite transparent so that trim color in the shell shows through). Code is far from complete, it is just a proof of concept. Pros: You can have aero anywhere in the window (the testcase only places a composite, but it could easily add other composite on left, right, bottom, etc) Works on cocoa and windows 7 If the platform does not support the bit, the user stills gets a functional UI does not have the client area problem (on windows) Cons: does not solve the menu bar problem on windows uses the last style bit available for SWT :(
Nice! (In reply to comment #30) > uses the last style bit available for SWT :( Wow. Time for SWT to only support 64bit. Kidding aside, seems like a worthy use of the last bit!
I looked at this a while back for Vista and experimented with a style bit SWT.GLASS (ie. the same idea). There were a serious problems with the code (needed 4 different hacks and all widgets needed to be double buffered). It appears that this is not the case for Windows 7. YAY! Here are my old hacks that "work" on Vista (but are not recomended): //BUFFERED PAINT FIXING AERO GLASS public static void main (String [] args) { OS.InitCommonControls(); TCHAR windowClass = new TCHAR (0, OS.IsWinCE ? "Dialog" : "#32770", true); int hwndShell = OS.CreateWindowEx ( 0 /*OS.WS_EX_LAYERED*/, windowClass, null, OS.WS_VISIBLE | OS.WS_OVERLAPPEDWINDOW, OS.CW_USEDEFAULT, 0, 200, 200, 0, 0, OS.GetModuleHandle (null), null); MARGINS pMarInset = new MARGINS (); pMarInset.cxLeftWidth = pMarInset.cxRightWidth = pMarInset.cyTopHeight = pMarInset.cyBottomHeight = -1; OS.DwmExtendFrameIntoClientArea (hwndShell, pMarInset); final int COLOR = 0xFD0000; DWM_BLURBEHIND pBlurBehind = new DWM_BLURBEHIND(); pBlurBehind.dwFlags = OS.DWM_BB_ENABLE; pBlurBehind.fEnable = true; // pBlurBehind.hRgnBlur = 0; OS.DwmEnableBlurBehindWindow(hwndShell, pBlurBehind); int bits = OS.GetWindowLong (hwndShell, OS.GWL_EXSTYLE); OS.SetWindowLong (hwndShell, OS.GWL_EXSTYLE, bits | OS.WS_EX_LAYERED); OS.SetLayeredWindowAttributes(hwndShell, COLOR, (byte)0, OS.LWA_COLORKEY); //PUT A TEXT WIDGET ON THE GLASS (double buffer to fix "cheese") final int hwndText = OS.CreateWindowEx ( 0, new TCHAR (0, "Edit", true), new TCHAR (0, "Some text", true), OS.WS_CHILD | OS.WS_VISIBLE | OS.WS_BORDER, 0, 0, 100, 32, hwndShell, 0, OS.GetModuleHandle (null), null); final int oldProc = OS.GetWindowLong (hwndText, OS.GWL_WNDPROC); Object windowProc = new Object () { public int windowProc (int hwnd, int msg, int wParam, int lParam) { switch (msg) { case OS.WM_ERASEBKGND: { //return 0; } case OS.WM_MOUSEMOVE: if ((wParam & OS.MK_LBUTTON) == 0) break; //FALL THROUGH case OS.WM_LBUTTONDOWN:{ //if (true) break; OS.DefWindowProc (hwnd, OS.WM_SETREDRAW, 0, 0); int code = OS.CallWindowProc (oldProc, hwnd, msg, wParam, lParam); OS.DefWindowProc (hwnd, OS.WM_SETREDRAW, 1, 0); OS.InvalidateRect (hwnd, null, true); OS.UpdateWindow(hwnd); return code; //break; } case OS.WM_PAINT: { System.out.println("WM_PAINT"); //if (true) break; int code = 0; PAINTSTRUCT ps = new PAINTSTRUCT (); int /*long*/ hDC = OS.BeginPaint (hwnd, ps); int width = ps.right - ps.left; int height = ps.bottom - ps.top; if (width != 0 && height != 0) { int /*long*/ [] phdc = new int /*long*/ [1]; int flags = OS.BPBF_TOPDOWNDIB; RECT prcTarget = new RECT (); OS.SetRect (prcTarget, ps.left, ps.top, ps.right, ps.bottom); int /*long*/ hBufferedPaint = OS.BeginBufferedPaint (hDC, prcTarget, flags, null, phdc); code = OS.CallWindowProc (oldProc, hwnd, msg, phdc [0], lParam); OS.BufferedPaintSetAlpha (hBufferedPaint, null, (byte)0xFF); OS.EndBufferedPaint (hBufferedPaint, true); } OS.EndPaint (hwnd, ps); return code; } } return OS.CallWindowProc (oldProc, hwnd, msg, wParam, lParam); } }; //PUT A BUTTON ON THE GLASS (not double buffered, black pixels are "cheesy") int hwndButton = OS.CreateWindowEx ( 0, new TCHAR (0, "BUTTON", true), new TCHAR (0, "Some text", true), OS.WS_CHILD | OS.WS_VISIBLE, 0, 50, 100, 32, hwndShell, 0, OS.GetModuleHandle (null), null); Callback newProc = new Callback (windowProc, "windowProc", 4); OS.SetWindowLong (hwndText, OS.GWL_WNDPROC, newProc.getAddress ()); final int oldProc2 = OS.GetWindowLong (hwndShell, OS.GWL_WNDPROC); Object windowProc2 = new Object () { public int windowProc (int hwnd, int msg, int wParam, int lParam) { switch (msg) { case OS.WM_NCPAINT: { //NON-CLIENT AREA HACK - not used if (true) break; //int /*long*/ hDC = OS.GetWindowDC (hwnd); int /*long*/ hDC = OS.GetDCEx (hwnd, 0, OS.DCX_WINDOW | OS.DCX_CACHE | OS.DCX_CLIPCHILDREN | OS.DCX_CLIPSIBLINGS); RECT rect = new RECT (); OS.GetWindowRect (hwnd, rect); OS.MapWindowPoints(0, hwnd, rect, 2); int hBrush = OS.CreateSolidBrush(COLOR); OS.FillRect (hDC, rect, hBrush); OS.DeleteObject(hBrush); OS.ReleaseDC (hwnd, hDC); break; } case OS.WM_NCCALCSIZE: { //NON-CLIENT AREA HACK - not used if (true) break; System.out.println(wParam); //if (wParam == 0) { int result = OS.CallWindowProc (oldProc2, hwnd, msg, wParam, lParam); int [] /*RECT*/ rect = new int [4] /*RECT()*/; OS.MoveMemory(rect, lParam, RECT.sizeof); rect[1] += 40; OS.MoveMemory(lParam, rect, RECT.sizeof); return result; //} //break; //return 0; } case OS.WM_ERASEBKGND: { //if (true) break; RECT rect = new RECT(); OS.GetClientRect(hwnd, rect); int hBrush = OS.CreateSolidBrush(COLOR); OS.FillRect (wParam, rect, hBrush); OS.DeleteObject(hBrush); return 1; //break; } case OS.WM_CLOSE: { OS.PostMessage (hwnd, /*OS.WM_QUIT*/ 0x12, 0, 0); break; } case OS.WM_COMMAND: { int code = OS.HIWORD (wParam); System.out.println(code); switch (code) { case OS.EN_CHANGE: { //REDRAW WHEN TEXT CHANGED TO DOUBLE BUFFER OS.InvalidateRect (hwndText, null, true); break; } } break; } } return OS.CallWindowProc (oldProc2, hwnd, msg, wParam, lParam); } }; Callback newProc2 = new Callback (windowProc2, "windowProc", 4); OS.SetWindowLong (hwndShell, OS.GWL_WNDPROC, newProc2.getAddress ()); int flags = OS.SWP_NOACTIVATE | OS.SWP_NOZORDER ; OS.SetWindowPos (hwndShell, 0, 200, 200, 300, 300, flags); OS.ShowWindow (hwndShell, OS.SW_SHOW); MSG msg = new MSG (); while (OS.GetMessage (msg, 0, 0, 0)) { OS.TranslateMessage (msg); OS.DispatchMessage (msg); } }
(In reply to comment #32) > I looked at this a while back for Vista and experimented with a style bit > SWT.GLASS (ie. the same idea). There were a serious problems with the code > (needed 4 different hacks and all widgets needed to be double buffered). It > appears that this is not the case for Windows 7. > > YAY! Yay indeed! Good to see you Steve. My suggestion regarding Vista is to have SWT pop up an error dialog that fills the screen with the following: "You have requested glass widgets on Vista. Please stop doing this, or consider contributing to the WPF port of SWT".
(In reply to comment #33) > (In reply to comment #32) > > I looked at this a while back for Vista and experimented with a style bit > > SWT.GLASS (ie. the same idea). There were a serious problems with the code > > (needed 4 different hacks and all widgets needed to be double buffered). It > > appears that this is not the case for Windows 7. Did you have the chance to try on Windows 7, did it work there ? > Yay indeed! Good to see you Steve. > My suggestion regarding Vista is to have SWT pop up an error dialog that fills > the screen with the following: "You have requested glass widgets on Vista. > Please stop doing this, or consider contributing to the WPF port of SWT". You can make the bit work for Winodws 7 and Cocoa only, if the application sets the bit in any other place the bit is cleared and a regular shell is created instead. I tried the snippet on XP, it doesn't look cool but things work. The application can always call getStyle() to check if the bit was clear and do something to improve the look. Steve, are you okay if the feature is only available on Windows 7 ?
Hi Felipe, we hope to have some more feedback for you later today. It seems that things are on the right track for getting this in though!
All, Status update for this patch - I'm currently pursuing idea provided by Felipe in Comment #30. I was trying to use the getToolbar() but I'm unable to work through widget's API. As soon as I have enough code to review will post it.
Created attachment 190332 [details] Aero Glass with borders
Created attachment 190333 [details] Aero Glass without borders
Created attachment 190334 [details] Sample application without header and WM_NCCALCSIZE
Felipe, there is significant difference in Glass when entire window is covered with it and where only partially. Take a look at two screenshots (Aero Glass with borders and Aero Glass without borders) I've uploaded to bug report. To resolve this, I've merged parts of the code from my earlier patch and made them available as a custom data (same way as in Bug #306039) Please look at the screenshot (Sample application without header and WM_NCCALCSIZE) - I also want to add features to remove icon and title and give developers possibility to extend client area, want it to ask if approach with custom data is acceptable in this case also.
Created attachment 190341 [details] Patch for Windows, based on Felipe's work This is patch with additional code added, based on Felipe's work. Pros: Widgets don't need to be adjusted to work correctly on glass. Cons: When shell has WS_EX_LAYERED bit set up there's no possibility to draw PNG alpha channel images using Label widget, only alpha=255 set on pixel will be transparent.
Created attachment 190344 [details] Patch for Windows - without WS_EX_LAYERED This time around patch that doesn't use WS_EX_LAYERED - uses black background. Pros - Ability to achieve alpha channel transparency. Suggested way of doing the Glass on Windows. Cons - Widgets needs to be adjusted, e.g. drawn with buffering. Issues with the patch: - Text widget fallback to not buffered when caret is moved by keyboard (did not happened on SWT 3.5) - Label widget with text is still drawn not correctly, needs to invest more time into DrawThemeTextEx - More widgets needs to be adjusted, e.g. Combo in read/write mode. Felipe, Bogdan, feedback would be appreciated.
Felipe, Bogdan, any feedback on this patches provided by Lukasz so that we can get this in before EclipseCon?
Felipe and Bogdan are both on vacation. I will try to provide some feedback by tomorrow.
Thanks Silenio! Will Felipe and Bogdan be back before EclipseCon?
Felipe will be back towards the end of next week. Bogdan will be back the week of EclipseCon.
Hi Felipe and Bogdan, Now that EclipseCon is over, any chance that we can get this into 3.7?
Felipe, Bogdan, any news on that feature? I know it's not perfect, but just need update status whether I need to invest more time into this patch or not Regards,
Hi Lukasz & Shawn, sorry for the delay. First, my patch was somewhat naive. The color key approach has no future. It is impossible finding a color that works for all cases. A common scenario that shows the weakness of this approach is text anti aliasing over the wrong background. The double buffer approach works for more cases, but it also has problems. First other widgets will need the fix (list, table, tree, toolbar, link to mention a few). The current patch is wrong: it calls the winproc twice; it does not double buffer user drawing (drawing in the gc during paint event); the code runs even when it is not needed (bad performance); etc. Other problem is controls that draw outside WM_PAINT. The patch fixes a couple places in text, but there are still some places in text that are broken, for example select text using the keyboard. Other controls will probably have this same problem. Another problem is transparency, this approach makes the entire control fully opaque. For example, try to add a label to the top composite in the example. Images also have limited support, I can't see how to make images work all the time. All said, I think you should try to go ahead with the double buffer approach, first fix your code (the bugs I listed above) and then try to use it for other controls. I'm not sure if we can get this done for 3.7 (sorry).
Felipe: Do we have a chance of getting this in? I am considering having us put more resources on it to push it through as my understanding is that we still have until April 29th until feature freeze. However, I am also concerned and a bit frustrated that something we have been quite diligent on has moved so little since October 26th. As far as I know the only slowness down from our end was the time we took 2 weeks to revamp a patch. Can we proceed?
Feature freeze is April 29, but this work requires new API and we are passed API freeze milestone. We can continue the work and ask for PMC approval to get it done for 3.7, but we are concerned that the current approach of double buffering every widget in WM_PAINT is quite scary and it could potentially destabilize eclipse. At least, the code has to written in such a way that nothing is affect if the Shell is not created with the new style bit to give us confidence that this feature would not break eclipse this late in the cycle. Sorry, it took us a lot of time to review the patch, but we have been busy with other emergencies. Hopefully we can have faster iterations from now on. Let's address the problems Felipe mentioned and see in what state we can get before the feature freeze.
Created attachment 192889 [details] Most recent patch Hi Silenio, Felipe, I'm attaching latest patch with small changes to the drawBuffered() method. That's something I had for some time now. Method on Shell, isGlassEnabled is used here to determine if actually Aero Glass is enabled on operating system level. This patch also contains new method DrawThemeTextEx with structure DTTOPTS, it will be used in Label to draw text on glass, either with or without special glow. Double buffering widgets is the only way I found to make them working correctly using standard black brush to determine glass areas. Maybe it's not perfect, but encompassing around buffered painting with transparency explicitly set is working fine for simple widgets, though I didn't found different approach to that (on side note, this also can be used to enable fade through animations). If you know different approach I'll be happy to look at it. Regarding Text widgets, I've opened bug #329292 to discuss problems with buffering paint methods, we can continue discussion about that particular widget there. Same goes for Label widget - bug #329293
Hi Lukasz, Your last patch didn't work for me (Eclipse would not apply it), anyhow here are my comments based on the diff: 1. We should focus on fixing the main parts of our solution: using double buffer for all widgets. At this point I would ask you to not add unused code to the patch as it makes harder to see the changes. 2. void drawBuffered(int /*long*/ hwnd, int /*long*/ lParam, int /*long*/ proc) { .. if (!getShell().isGlassEnabled()) { return; } I do not think this is the correct code. The correct code should clear SWT.TRIM_FILL from Shell when there is no support in the OS (as the application code also needs to know that, i.e getStyle()). Therefore, isGlassEnabled() should only be called during the constructor of Shell. For example, in Shell#checkStyle(), SWT.TRIM_FILL is clear is isGlassEnabled()==false; (note that when you move the check to Shell#checkStyle() you will not need the method anymore, just inline the code). 3. void drawBuffered(int /*long*/ hwnd, int /*long*/ lParam, int /*long*/ proc) { if ((style & SWT.TRIM_FILL) == 0) { return; } Fine, but we should make sure that TRIM_FILL control can not be parented by a control without TRIM_FILL. This again should be checked during the constructor during checkStyle(). 4. you still have the double buffer code in callWindowProc(). That is not the correct place for it. If the code stays there, it has all these problems: -the code is calling the default window proc twice (drawing the widget twice). -you are only double buffering a few widgets -drawing done in SWT.Paint is not being double buffered -does not integrate well with the framework The problem here is that SWT does not have a single path where all WM_PAINT go through. Most controls use Widget#wmPaint, but there are many exceptions: Link/ExpandBar, Composite, Table/Tree. 5. Toolbar, did you ever got ToolBar to work on the trim and outside of the trim ? To move forward, Lukasz you can work on (1), (2), (3), and (5). Silenio and I will see how you can refactor SWT so we can add double buffer (and custom drawing) to all controls more easily (4).
Hi Felipe, I'll start my work on this right away. Regarding #2 what about a situation that user starts application while having Aero classic style without Glass and then switch to Aero with glass? Do we cover this scenario right now, or should we move it to later stage? Regarding #5 I've tested and Toolbar widget works without any problems as long it has PNG image. Still need to look more into it. What do you mean by trim? Regards
(In reply to comment #54) > Hi Felipe, > I'll start my work on this right away. > Regarding #2 what about a situation that user starts application while having > Aero classic style without Glass and then switch to Aero with glass? Do we > cover this scenario right now, or should we move it to later stage? later, I suppose the system sends a message (like WM_SETTINGCHANGE) so we should be able to fix. > Regarding #5 I've tested and Toolbar widget works without any problems as long > it has PNG image. Still need to look more into it. What do you mean by trim? > Regards Toolbar has to look nice in trim area (the glass area), and in the client area (opaque). Can you add a screenshot o the toolbar working on the trim area and in the client area (if you use my AeroControlExample snippet, trim is the top composite, the client area any child of the bottom composite) ? What is the problem with image? in Label (and other widgets) I noticed that images with alpha (like PNG) would look good without double buffering (using double buffer would cause the transparent pixels to be black). On the other hand, opaque images (with black pixels) or images with transparent mask would only look good with double buffer.
(In reply to comment #55) > (In reply to comment #54) > > Hi Felipe, > > I'll start my work on this right away. > > Regarding #2 what about a situation that user starts application while having > > Aero classic style without Glass and then switch to Aero with glass? Do we > > cover this scenario right now, or should we move it to later stage? > > later, I suppose the system sends a message (like WM_SETTINGCHANGE) so we > should be able to fix. Correct, there is speciale message WM_DWMCOMPOSITIONCHANGED for that. > > > Regarding #5 I've tested and Toolbar widget works without any problems as long > > it has PNG image. Still need to look more into it. What do you mean by trim? > > Regards > > Toolbar has to look nice in trim area (the glass area), and in the client area > (opaque). Can you add a screenshot o the toolbar working on the trim area and > in the client area (if you use my AeroControlExample snippet, trim is the top > composite, the client area any child of the bottom composite) ? Sure I can do it, expect tomorrow morning, as my day ends ;-) > What is the problem with image? > in Label (and other widgets) I noticed that images with alpha (like PNG) would > look good without double buffering (using double buffer would cause the > transparent pixels to be black). On the other hand, opaque images (with black > pixels) or images with transparent mask would only look good with double > buffer. The problem would be with the fact, that glass requires drawing with full alpha channel, and if image is in PNG 24bit with alpha channel then everything is fine and proper methods are used to draw it, in other cases old methods are used (at least this is my understanding of the subject) At the beginning of this development, I've experimented with JFace Wizards and found that problem (with images) to be rather annoying - when some images had proper alpha channel, some did not.
Created attachment 193178 [details] Toolbar on glass screenshot This is from my "example of glass usage" application written in Eclipse RCP - it contain two toolbars, both doesn't show text, though. Still needs to investigate that particular case, but as you can see, buttons are rendered correctly with alpha transparency (border of button and images)
(In reply to comment #57) > Created attachment 193178 [details] > Toolbar on glass screenshot Right, I suppose you set black background for the toolbar on the top (so it is transparent) and you did not set black background for the toolbar on the bottom. Do you know how to show text and image in the toolbar on the top ? If you keep it transparent the text looks bad (can't read the text), if you make it opaque you can read the text but looks ugly (big gray background). In Label you sort of solve the problem by making the label transparent when an image is set. Check and Radio buttons (with image) have the same problem as ToolItem: how to show an image and text on a widget.
Hi Felipe, I think that's fundamental problem with Aero Glass itself - it's not capable of accommodating every possible widget out there. I've been searching for an app that would hold all above widgets (toolbar with both text and image, checkbox and radiobox) but failed to do that - look at the newest IE from Microsoft, they basically recreated toolbar even though no text is used there. As for Toolbar, you're corrected, my example has TRIM_FILL set for Toolbar widget, hence the correct drawing. Label widget can be enhanced further if we change drawing method that is suitable for Aero. Not sure if it can be done in Toolbar or any other widgets that are drawn by operating system without sacrificing transparency of the widget (i.e. using the double buffer method on entire bounds). Just wondering, any suggestions regarding #4 from your list? I've already finished #2 and #3 and will post a patch tomorrow morning when I clean up other work done on that code.
Hi Lukasz, For the problem I presented in comment #58 (control that need show both text and image at the same time, like toolbar and radio buttons) I see two solution: 1. make them opaque. For example, when the toolbar only has images it draws transparent. The second it has text the entire toolbar becomes opaque. 2. Custom draw. Other problem that I tried to isolate in the past is that not all images will work on transparent controls. I tried to enable/disable tranparency based on the transparency type of the image (alpha,mask,pixel) but I didn't finish the work. The solutions I see are: -Only support aero transparency for controls with alpha transparency images (easy in the short term). -Convert other types of image transparency (mask,pixel) to alpha. did you do any work on this area ? Anyhow, I did some work on #4 but didn't quite finished... I think moving foward we should first release the core bits of the framework (#2, #3 #4) and then start fixing the controls one by one. Label - custom draw Text - extra redraws etc
Created attachment 194175 [details] Latest glass patch Hi Felipe, I'm attaching latest glass patch, it contains changes as request for #1, #2 and #3 comment. Checking for parent is not possible in widget's checkStyle() so I've move it to Control's constructor, if wrong, please suggest different place - I've been trying to not write this piece of code in every widget. Even though I've removed some stuff according to #1, I've left DrawThemeTextEx and accompanying DTTOPTS class so that we can go forward with Label widget - if this makes some problems I can remove it and provide updated patch. Regards,
Hi Felipe, Any news regarding double buffering? Regards,
(In reply to comment #62) > Hi Felipe, > Any news regarding double buffering? > Regards, Sorry Lukasz, we are on the final weeks for Eclipse 3.7 and fixing the last bugs we can. Soon we will have more time to work on new features. Maybe I start a branch on CVS and we start releasing some of the code to it ? Once we have something working we can merge it to HEAD (but only after 3.7 is declared). Does that make sense ?
Hi Felipe, That sounds like a great idea to get this into a branch now so that the patch isn't hanging around. Also, this will make it easier to people who may want to consume a custom build of SWT with glass support for RCP apps. Can you let us know when you have the branch created and what the branch name is?
(In reply to comment #64) > Hi Felipe, That sounds like a great idea to get this into a branch now so that > the patch isn't hanging around. Also, this will make it easier to people who > may want to consume a custom build of SWT with glass support for RCP apps. Can > you let us know when you have the branch created and what the branch name is? done, checkout org.eclipse.swt org.eclipse.swt.win32.win32.x86 branch: AERO_WORK Compare the HEAD of the branch against vesion Root_AERO_WORK I tried to release an minimal version to get us started. Please, see the compare and try to understand the code let me know if you have any doubts or suggestions. I know that my part of the work is not final, but I think it is good enough for now. The next step after this is to choose a task (custom draw for label, fixes for text) and finish that. Sends me the patches and I will merge them to the branch, but one at the time. Sounds good ?
Thanks Felipe! Lukasz, do you have a patch against this branch to support labels or text fields?
Created attachment 201026 [details] Label widgets with Aero/Glass look. Screenshot of a shell containing 3 label widgets, with the SWT.TRIM_FILL flag set to true.
Created attachment 201027 [details] Label widgets without Aero/Glass look. Screenshot of a shell containing 3 label widgets, with the SWT.TRIM_FILL flag set to false.
Created attachment 201028 [details] Patch for Aero/Glass Label widgets.
I'm a member of the Tasktop development team, and I've just submitted a patch and a couple of screenshots to demonstrate some of the progress we've made recently in adding Aero/Glass support to SWT (and to the Label widget in particular). Some points I'd like to bring up: - the demo screenshots and patch were built from the AERO_WORK branch which Felipe created for this purpose - the patch has a dependency on OS.DrawThemeTextEx(), which is not currently available in the swt.dll JNI library (we've been using our own JNI implementation as a temporary workaround) - the only difference between the 2 screenshots is the use of the SWT.TRIM_FILL flag in the construction of the root Shell object - the Label.IMAGE_AND_TEXT was set to TRUE to demonstrate that images and text could be drawn side-by-side (this seemed to be a theme in earlier conversations) - the patch has been tested successfully with PNG images, but GIF and BMP image formats are known to be problematic - there is a lot of commonality between wmDrawChild and wmBufferedPaint, which we may want to consider refactoring - drawing of Label separators is not completed yet (code is currently only drawing white lines, still a WIP) I believe the patch is a good proof of concept of the approach that Felipe was suggesting, and we ask that you consider the patch for eventual merge onto the SWT branch. Your review and feedback would be greatly appreciated. Our intention is to also start addressing other widgets - is there a preference for the order in which we should start tackling the other widgets? I think the Text widget makes a logical next candidate. Also, I notice that there are other tasks (329292 329293 329299), but I didn't see much activity on them. Is the preference to post future work against this task, or to child tasks specifically for the widget under development? One final thing, since I'm relatively new to the list, it doesn't hurt to get the IP issues clarified, so this is my confirmation that I, Raymond Lam: - Authored the code. - Have a right to contribute the content to Eclipse. - Contribute the content under EPL.
Hi Raymond, I have released your changes here http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=0021915c8f9ff5617daec62f9ed0f26e6f901e98 and I have also released a few changes on top, mostly so that I could compile and run the changes in my machine. please see the changes: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=922a78f2ff87424ff005089c76b83b8d77aaf4c8 The code is still in a very early stage, so I will not post too many question and suggestions (as I believe you know places that need to changed in the code), but here are few ones: - Label#wmBufferedPaint is being called from Control#WM_PAINT and Label#wmDrawChild - ControlPanelStyle; and CompositedWindow::Window; where did you get that ? btw, see Display#hButtonTheme for an example on how we cache hTheme pointers. Anyhow, the final results is not working for me. the code is running but nothing is drawn.
Hi Felipe, Thanks for your commits and feedback! 1) I should have a better abstraction for wmDrawChild and WM_PAINT shortly. 2) The use of CompositedWindow::Window; was copied from this MSDN example here on the DWM http://msdn.microsoft.com/en-us/library/bb688195%28VS.85%29.aspx and ControlPanelStyle; is one of the styles taken from AeroStyle.xml. I haven't finalized any of these window class choices yet, but they seem to work for now. Thanks for the pointer on theme caching - I'll see if I can use the same approach. With regards to it not painting for you, I couldn't tell from your commits if you had added support for DrawThemeTextEx in the swt.dll. Can you confirm that this was added into your build? (I didn't see any changes added to os.c) I could try to get those changes to os.c to you, but I haven't been able to build swt.dll successfully so I wouldn't be able to verify my changes prior to handoff.
(In reply to comment #72) > With regards to it not painting for you, I couldn't tell from your commits if > you had added support for DrawThemeTextEx in the swt.dll. Can you confirm that > this was added into your build? (I didn't see any changes added to os.c) I > could try to get those changes to os.c to you, but I haven't been able to build > swt.dll successfully so I wouldn't be able to verify my changes prior to > handoff. Sorry Felipe, I was looking at the wrong link - I do see your changes to os.c. I'll see if we can replicate your problem using these changes.
Raymond pointed the reason why the code was not working for me, I was missing DttOpts.dwSize = DTTOPTS.sizeof; (which he had in his native code). This commit fixes this problem: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=0f39d125f39ba79bd8dd8a8339f955fa33302ab5 Raymond, are you able to compile/run using the same code as I ? Note that if you use a newer version of the SDK the compiler will complain that the struct DTTOPTS is defined twice.
> This commit fixes this problem: > > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=0f39d125f39ba79bd8dd8a8339f955fa33302ab5 Thanks Felipe. This works for me now too. > > Raymond, are you able to compile/run using the same code as I ? Yes, in fact I've grabbed your latest commit and am now building both the native dll and java code against your code base. I'll be using that as my reference point for my next patch. > Note that if you use a newer version of the SDK the compiler will complain that > the struct DTTOPTS is defined twice. I've decided to build against the 2003 Server SDK for the time being. I didn't want to introduce too many variables all at once.
Created attachment 202839 [details] Patch for Control#drawBackgroundBufferred and Label#wmDrawChild
Hi Felipe, I've made some enhancements to reduce the amount of duplicate code that used to exist in Label#wmDrawChild and Label#wmBufferredPaint: 1) added Control#drawBackgroundBufferred so that other Controls have the Glass look painted as background 2) modified plain text Labels to be owner-drawn in the case where Glass is turned on 3) removed Label#wmBufferredPaint since almost all drawing is owner-drawn (except for above) and was already being done in Label#wmDrawChild 4) added Label#drawBufferredText so that text is rendered correctly by Label#wmDrawChild when Glass is turned on I've also looked into the existing themes which are being cached in Display, but none of them seemed to return a proper glow size. Hence, I've left the themes the way they are for now in Label, and was wondering if you had any preference as to whether we change them or add new ones to the set already being cached in Display. Ray
So who is going to be the first to suggest porting Eclipse/SWT to Metro...
Hi Raymond, I saw your code and I released it about one week ago but I forgot to update the bugzilla. Sorry The code is here: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=a07ed48fb9192d9cee2a365d410ac695fd1c5098 Sure you made the code much clear, good work. My only concern is that one of the main changes (that simplified things) is in Control#drawBackground () That affects all control not only Text. Besides, in Control we already had a hack in getBackgroundPixel() to draw the background differenctly when SWT.TRIM_FILL is set. I guess we need to fix a few more controls to understand better what we need to do, after that we can decided how to change the draw framework. What is next in your list ?
(In reply to comment #78) > So who is going to be the first to suggest porting Eclipse/SWT to Metro... i guess you sort of did ;-)
Hi Felipe, Yes, as I was adding Control#drawBackgroundBuffered, I could see that there were potentially going to be quite a few changes, but I wanted to keep my code impact minimal, and hence stopped at just that one method. Next on the list was going to be Combo and Text. Both seem to have problems displaying the text and in showing highlighted text. I've been looking into these painting problems, and hoping to have them addressed by early next week. Ray
(In reply to comment #80) > (In reply to comment #78) > > So who is going to be the first to suggest porting Eclipse/SWT to Metro... > > i guess you sort of did ;-) Not sure I'm willing to go down on record as the person to suggest that. Metro has some neat aspects to it, but I'm not sure that the IDE was the primary interaction target ;) RT @mik_kersten: LOL Info on tiles may be good for tablets. Can't see much benefit on desktop RT @SimoRoth: Windows 8 25 years too late: http://bit.ly/nkrjCx
Created attachment 204431 [details] Patch for Aero/Glass Text widgets.
Created attachment 204432 [details] Patch for Aero/Glass Combo widgets.
Hi Felipe, I've submitted a couple of patches which improve how the Text and Combo widgets are rendered with glass turned on. It's a similar approach between the two widgets, where the native Edit control is not alpha channel aware and hence we have to ask it to render in our own buffered DC, where we then later force an opaqueness of 100%. The other trick is that the Edit control performs many of its paint operations outside of WM_PAINT, and the only reliable way to detect when a paint is about to happen is handle the WM_CTLCOLOREDIT message that is issued by the Edit control when it's about to redraw itself. If you would prefer that this be refactored out into a helper class, let me know - it would be a pretty easy change to make. Ray
Hi Raymond, I have: - released all the code to the branch - released all the native code to master (so we can have them in the binaries) - fixed native code for wince - fixed 64bits bugs - merged with master If you pull the lasted from eclipse.platform.swt (AERO_WORK) and eclipse.platform.swt.binaries (master) you should be able to run the exact same code as I'm running. I have tested the code for a bit, but not much. As for the code, I didn't understand parts of it, but I'll go over again with more time (I wanted to merge everything first) and describe it to you.
Created attachment 205668 [details] Patch for new Aero/Glass utility: BufferedPainter.
Created attachment 205669 [details] Patch for Aero/Glass ProgressBar widget.
Created attachment 205670 [details] Patch for Aero/Glass Link widget.
Comment on attachment 205670 [details] Patch for Aero/Glass Link widget. Please ignore - wrong version of the file was committed.
Created attachment 205672 [details] Patch for Aero/Glass Link widget.
Hi Felipe, thanks for merging everything onto the branch. There's a couple of new widgets that I've just added: the ProgressBar widget and the Link widget. The ProgressBar was pretty simplistic, as all I needed to do was make the corner pixels transparent in order to give the widget a more rounded look. The Link widget was a little more involved, as I needed to revert to the legacy mode of performing all the painting through the TextLayout class instead, giving us the control over how text gets rendered against a Glass surface. Next up is Button, which I'm targeting for the end of the week. Thanks, Ray
Created attachment 205771 [details] Fix drawBackgroundBuffered to paint cleanly.
Created attachment 205772 [details] Patch to add GetThemeBitmap to native swt dll.
Created attachment 205773 [details] Patch for Aero/Glass Button widget.
Hi Felipe, I've submitted the patch for the Button widget. I needed to add a GetThemeBitmap to the native dll in order support some of the pixel calculations during painting. The Button widget supports SWT.PUSH, SWT.TOGGLE, SWT.RADIO and SWT.CHECK. Last up is Toolbar / ToolItem.
Hi Raymond I pushed the progress bar fix http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=c0ef669c9afa5a9ff8ccbbdaa0c41939cd22c383 and the drawBackgroundBuffered fix http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=f4c41647aaa3faa2675335f7e68bd34a028ac49a (it would be good if you explained why this change was needed) The GetThemeBitmap native I did not push (to master) because master is closed for M3 this week. The button widget fix I did not push because depends on the GetThemeBitmap. The utility BufferedPainter I did not push because we try to avoid utility classes in SWT. It seems to be that Control is correctly place for the two methods you need to share. The Link patch I didn't push because it felt to hacky (in particular adding a public method to TextLayout). Can't we fix this problem any other way ? Maybe you should go ahead and finish ToolBar and ToolItem, so we know what need to be done for all controls. After that we need a big clean up pass to remove all the hacks.
Created attachment 206246 [details] Patch for Aero/Glass ToolBar and ToolItem widgets
Hi Felipe, My latest patch adds Aero/Glass support to the ToolBar and ToolItem widgets. As per your suggestion, I've also removed the utility class, and moved the 2 methods from BufferedPainter into Widget. You had originally suggested Control, but I had to step up to Widget since ToolItem is not a Control. As for the drawBackgroundBuffered fix, this was needed because device contexts would sometimes be recycled by the OS, and if we did not erase the contents there would be ghost artifacts left behind from previous paint operations when dragging the window from off screen. I'll attach an image to show the effects I was seeing. I'll look into another way of patching the Link widget. The challenge here is that all of the drawing for the Link widget is actually performed by TextLayout, and we need to communicate the status of SWT.TRIM_FILL into TextLayout and it needed the abilities of BufferedPainter to accomplish its drawing. I'll work on an alternative approach for Link widget. This completes the inventory of widgets that we were targeting for this pass. If you could provide an assessment of outstanding issues, and how you would like to see the code cleaned up, that would be great. Along those lines, one of the issues that our team has identified is that the Shell currently turns glass on for the entire client area, and that we need a way to control the size of the glass borders so that a Shell can have both glass and non-glass areas. Right now, the dimensions we are passing into DwmExtendFrameIntoClientArea are maxed out, and it would be preferable to specify the dimensions which are passed into that OS call.
Created attachment 206247 [details] Screenshot showing ghost artifacts from incorrect drawBackgroundBuffered implementation.
(In reply to comment #94) > Created attachment 205772 [details] > Patch to add GetThemeBitmap to native swt dll. I released this patch to master and the branch, Notes - you probably don't need to include in the patch the files that are auto-gen - GetThemeBitmap() the last parameter is a pointer (HBITMAP *), so in java it is a int /*long*/
(In reply to comment #95) > Created attachment 205773 [details] > Patch for Aero/Glass Button widget. Release to the branch Notes: - Number of 64 bits problems, please use the SWT Tool the report 64bit bugs Select org.eclipse.swt, context menu, SWT Tools, check "Report 64-bit Problems" - Remove the references to BufferedPainter. - Code format does not conform SWT standart for example, we do not use int myfunc() { //body } We use: int myfunc () { //body } Sorry that we don't have a formal documents describing our standard, we need to look at the exist code the follow the same pattern... The methods should alphabetically ordered Avoid if (some==true) {} and if (some==false) {} tests Qualify an instance var with this. only when necessary we don't usually comment internal methods (specially not with /**) we don't add comments of this type: // see if the button is in a disabled state if (getEnabled () == false) { if ((style & SWT.RADIO) != 0) { // radio button return (dwCheckState == OS.BST_CHECKED) ? OS.RBS_CHECKEDDISABLED : OS.RBS_UNCHECKEDDISABLED; } else { // checkbox button return (dwCheckState == OS.BST_CHECKED) ? OS.CBS_CHECKEDDISABLED : (dwCheckState == OS.BST_UNCHECKED) ? OS.CBS_UNCHECKEDDISABLED : OS.CBS_MIXEDDISABLED; } } should just be: if (!getEnabled ()) { if ((style & SWT.RADIO) != 0) { return (dwCheckState == OS.BST_CHECKED) ? OS.RBS_CHECKEDDISABLED : OS.RBS_UNCHECKEDDISABLED; } else { return (dwCheckState == OS.BST_CHECKED) ? OS.CBS_CHECKEDDISABLED : (dwCheckState == OS.BST_UNCHECKED) ? OS.CBS_UNCHECKEDDISABLED : OS.CBS_MIXEDDISABLED; } } Avoid unnecessary brackets, for example boolean captured = (OS.GetCapture() == this.handle); should be boolean captured = OS.GetCapture () == handle; Don't mix tab and whitespace, we only use tab to indent the code. Dont use String.toCharArray() In widget: - public int getThemeGlowSize () { - public void drawBufferredText it can't be public... constant OS.DTT_TEXTCOLOR was not exported etc
commit for Glassify the Button widget. http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=afa64269dcd7778f6eb01e1cb2d17a3455e7a43e commit for Toolbar http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=4b8def71d48644f2eecf0ad1acd6947c48451ff6 commit for some fixes I added: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=ba80227e53a47072a77de932fddc6051e99a1178 Note I didn't have the time to over all the places, nor test the code for leaks and other problems.
(In reply to comment #99) > My latest patch adds Aero/Glass support to the ToolBar and ToolItem widgets. As > per your suggestion, I've also removed the utility class, and moved the 2 > methods from BufferedPainter into Widget. You had originally suggested Control, > but I had to step up to Widget since ToolItem is not a Control. We could have put the drawing code in Toolbar, there is no hard reason to put the code in ToolItem. Anyway, no big deal. > > As for the drawBackgroundBuffered fix, this was needed because device contexts > would sometimes be recycled by the OS, and if we did not erase the contents > there would be ghost artifacts left behind from previous paint operations when > dragging the window from off screen. I'll attach an image to show the effects I > was seeing. I released this fix, right ? > I'll look into another way of patching the Link widget. The challenge here is > that all of the drawing for the Link widget is actually performed by > TextLayout, and we need to communicate the status of SWT.TRIM_FILL into > TextLayout and it needed the abilities of BufferedPainter to accomplish its > drawing. I'll work on an alternative approach for Link widget. ok > > This completes the inventory of widgets that we were targeting for this pass. > If you could provide an assessment of outstanding issues, and how you would > like to see the code cleaned up, that would be great. See my two last comments, please try to clean up the rest of the code based in some of the changes I did in my last commit. > Along those lines, one of the issues that our team has identified is that the > Shell currently turns glass on for the entire client area, and that we need a > way to control the size of the glass borders so that a Shell can have both > glass and non-glass areas. Right now, the dimensions we are passing into > DwmExtendFrameIntoClientArea are maxed out, and it would be preferable to > specify the dimensions which are passed into that OS call. Once upon the time you had a setData hack for that, right ? I'm not sure how to express this API in a cross platform way...
(In reply to comment #104) > (In reply to comment #99) > > My latest patch adds Aero/Glass support to the ToolBar and ToolItem widgets. As > > per your suggestion, I've also removed the utility class, and moved the 2 > > methods from BufferedPainter into Widget. You had originally suggested Control, > > but I had to step up to Widget since ToolItem is not a Control. > > We could have put the drawing code in Toolbar, there is no hard reason to put > the code in ToolItem. Anyway, no big deal. > Yes, true enough. I was thinking that ToolItems should be able to paint themselves, and hence wanted to avoid centralizing all the painting code in ToolBar. As you say, it's not a big deal and we can move it there later if need be. > > > > As for the drawBackgroundBuffered fix, this was needed because device contexts > > would sometimes be recycled by the OS, and if we did not erase the contents > > there would be ghost artifacts left behind from previous paint operations when > > dragging the window from off screen. I'll attach an image to show the effects I > > was seeing. > > I released this fix, right ? > Yes you did, as part of comment 97 through this commit http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=f4c41647aaa3faa2675335f7e68bd34a028ac49a > > I'll look into another way of patching the Link widget. The challenge here is > > that all of the drawing for the Link widget is actually performed by > > TextLayout, and we need to communicate the status of SWT.TRIM_FILL into > > TextLayout and it needed the abilities of BufferedPainter to accomplish its > > drawing. I'll work on an alternative approach for Link widget. > > ok > > > > > This completes the inventory of widgets that we were targeting for this pass. > > If you could provide an assessment of outstanding issues, and how you would > > like to see the code cleaned up, that would be great. > > See my two last comments, please try to clean up the rest of the code based in > some of the changes I did in my last commit. Thanks for all those pointers - I will make a complete pass over all the code changes and make sure they conform as per your stated standards. > > > Along those lines, one of the issues that our team has identified is that the > > Shell currently turns glass on for the entire client area, and that we need a > > way to control the size of the glass borders so that a Shell can have both > > glass and non-glass areas. Right now, the dimensions we are passing into > > DwmExtendFrameIntoClientArea are maxed out, and it would be preferable to > > specify the dimensions which are passed into that OS call. > > Once upon the time you had a setData hack for that, right ? > I'm not sure how to express this API in a cross platform way... Yes, that was back from several months ago during our very early prototyping days. I can try to bolt on an approach that rides on top of setData, and we can review afterwards.
Created attachment 207424 [details] Patch for Aero/Glass Link widget. Taking a new approach with the Link widget - eliminated the public method, and added a new constructor instead. The coupling between Link and TextLayout is implemented through a new interface defined in TextLayout.
> > I'll look into another way of patching the Link widget. The challenge here is > > that all of the drawing for the Link widget is actually performed by > > TextLayout, and we need to communicate the status of SWT.TRIM_FILL into > > TextLayout and it needed the abilities of BufferedPainter to accomplish its > > drawing. I'll work on an alternative approach for Link widget. Please review the new patch for the Link widget. The package separation between Link and TextLayout required that some kind of minimalistic coupling be provided between the 2 classes, and instead of the old approach of adding a public method on TextLayout, I've added a new constructor and new interface definition instead. The new constructor signature provides the ability to specify an optional implementation of TextLayout#DrawRunText (the new interface), and this allows all of the drawing to take place inside of the Link widget where it really belongs. This change to the Link widget makes it much more consistent with how the other widgets have been enhanced to support Aero/Glass, and it also eliminated the need for TextLayout to be aware of any Aero/Glass issues at all.
Created attachment 207544 [details] Screen shot demonstrating glass and non-glass regions on the same Shell.
Created attachment 207545 [details] Fix glass detection logic in Button widget.
Created attachment 207546 [details] Patch to allow non-glass regions to exist on a glass-enabled shell.
Hi Felipe, I've submitted a couple of patches - one is really minor and simply corrects the logic used in detecting glass or non-glass inside of Button#getBufferredPaint. The other patch provides a way for developers to define non-glass regions on a glass-enabled Shell. As discussed, this was done through Shell#setData, and I've attached a screenshot demonstrating this capability. The approach is by no means perfect, as there has to be a tight awareness between the Shell's glass coordinates (passed in through Shell#setData) and the boundaries of where the expected glass vs. non-glass areas are to intersect. This places a bit of burden on the UI developer, so while it's not convenient it's not fatal either. Let me know what you think.
Forgot to mention, I'll be performing a general round of code cleanup (as per your suggestions in your last message) and hopefully once that is complete we can look at moving this work to the next level and consider what options we have for releasing the Aero/Glass capabilities to the community. Thanks!
Created attachment 207578 [details] Miscellaneous cleanup of previous Aero/Glass patches This patch performs a general cleanup of the code that has been accumulated on the AERO_WORK branch: -fix 64-bit errors -sort methods in alphabetical order -use theme caching provided by Display -conform to de facto SWT coding standards (brackets, comments, whitespace, etc.)
(In reply to comment #109) > Created attachment 207545 [details] > Fix glass detection logic in Button widget. http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=092a2269ca277410b54736129c2ad6cee19fe1eb
(In reply to comment #110) > Created attachment 207546 [details] > Patch to allow non-glass regions to exist on a glass-enabled shell. http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=0748aaad9afe20d4a1e568e8b6bdd9eacd347409 review http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=1c89c90eebf251a5fbcc2833e8758ef654d57e4e remove any part of the code that is APIs (also note that the type MARGINS is internal, could never be part of our API) add the method in alphabetic order.
(In reply to comment #106) > Created attachment 207424 [details] > Patch for Aero/Glass Link widget. > > Taking a new approach with the Link widget - eliminated the public method, and > added a new constructor instead. The coupling between Link and TextLayout is > implemented through a new interface defined in TextLayout. The problem is that you are adding new API to TextLayout. This is actually worse than the method, look at our API: - we avoid interfaces. - we don't use public internal types. Besides, the API is wrong. It takes win32 only type (hFont, RECT). It allows the rendering to be overwrite but not the measuring, etc It is just a hack to fix Link. Right ? Note that we can't have hacks in the API. I think you could use the TextLayout to find the bounds of each text run and draw it using DrawThemeTextEx(), note that you need to draw the entire text everytime using clipping to preserve ligatures and clusters. You will need to draw the underline too, but that is the easy part.
(In reply to comment #111) > Hi Felipe, > > I've submitted a couple of patches - one is really minor and simply corrects > the logic used in detecting glass or non-glass inside of > Button#getBufferredPaint. > > The other patch provides a way for developers to define non-glass regions on a > glass-enabled Shell. As discussed, this was done through Shell#setData, and > I've attached a screenshot demonstrating this capability. > > The approach is by no means perfect, as there has to be a tight awareness > between the Shell's glass coordinates (passed in through Shell#setData) and the > boundaries of where the expected glass vs. non-glass areas are to intersect. > This places a bit of burden on the UI developer, so while it's not convenient > it's not fatal either. > > Let me know what you think. I don't really like it, but right now I don't have a better suggestion :(
(In reply to comment #113) > Created attachment 207578 [details] > Miscellaneous cleanup of previous Aero/Glass patches > > This patch performs a general cleanup of the code that has been accumulated on > the AERO_WORK branch: > -fix 64-bit errors > -sort methods in alphabetical order > -use theme caching provided by Display > -conform to de facto SWT coding standards (brackets, comments, whitespace, > etc.) It failed to apply the patch (because of SWT/Link/Shell) Please send an updated patch and I'll push it. If you prefer to finish Link first that is okay too. (I was thinking of my suggestion in comment 116, it should not be too hard...)
Created attachment 208018 [details] Miscellaneous cleanup of previous Aero/Glass patches (re-submission) Here is a resubmission of attachment 207578 [details] since the first submission didn't take because of Link. This patch performs a general cleanup of the code that has been accumulated on the AERO_WORK branch: -fix 64-bit errors -sort methods in alphabetical order -use theme caching provided by Display -conform to de facto SWT coding standards (brackets, comments, whitespace, etc.)
(In reply to comment #118) > It failed to apply the patch (because of SWT/Link/Shell) > > Please send an updated patch and I'll push it. > If you prefer to finish Link first that is okay too. > (I was thinking of my suggestion in comment 116, it should not be too hard...) Hi Felipe, Thanks for your feedback - I've updated and resubmitted the patch as requested. I'll take another stab at Link now - your suggested approach seems like a good way to tackle it.
http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=093c5424ad9dd621b6f299675f1805e3a472a008 http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=7bad1d999cfc6ba842660106d4818e34fea28a50 Thanks Raymond, getting much better!
Created attachment 208949 [details] Patch for Aero/Glass Link widget. Hi Felipe, Sorry for the delay - my iMac crashed in early December, and then holidays became a big distraction. Hope you had an enjoyable holiday season. I took your suggestions for the Link widget, and now have a solution which avoids any messing around with TextLayout at all. You'll find that all of the code changes are in the Link class itself (as it should be, as per your comments) and is now implemented as a typical Glass-enabled SWT widget. Let me know what you think.
Sorry to hear about your mac. The code is moving to correct direction, but as is I suppose it does not support text wrapping, is that right ? the idea is to use TextLayout to find out where to draw each text run (so that wrapping gets taking care for you).
I did try to give TextLayout a shot, but I couldn't find an easy way to get access to all the run information. And since I didn't want to alter TextLayout, I ended up brewing my own Link#computeBufferedTextOffsets method. It looks like I might be able to deduce the information through a combination of TextLayout#getRanges and TextLayout#getLineOffsets. Do you think such an approach will be appropriate?
(In reply to comment #124) > It looks like I might be able to deduce the information through a combination > of TextLayout#getRanges and TextLayout#getLineOffsets. Do you think such an > approach will be appropriate? That is what I thought, note that you don't need the ranges (it should be the same as the offsets[] that you already have in Link), I think you will need to use getBounds(int,int) to find out where each text run goes. You will need to match the ranges to the line ranges but that should not be too hard.
Created attachment 209415 [details] Patch for Aero/Glass Link widget.
Created attachment 209416 [details] Bug fix - pass the correct flags into DrawThemeTextEx from Label widget.
Felipe, turns out you're hunch was right - I'm now using the structures from TextLayout to draw multi-line Link widgets. It also removed the need for my own Link#computeBufferedTextOffsets method, so that's no longer there. Note that I've had to turn off the underlying SysLink control as it was causing a lot of intermittent flicker and this is why you'll see a lot statements like this: if (OS.COMCTL32_MAJOR >= 6 && !getBufferredPaint()) I also submitted a fix to a minor bug that I found along the way. Please let me know what you think.
(In reply to comment #126) > Created attachment 209415 [details] > Patch for Aero/Glass Link widget. thank you http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=7a1299e262185c88e376cd20f10620839ad6c37d
(In reply to comment #129) > (In reply to comment #126) > > Created attachment 209415 [details] > > Patch for Aero/Glass Link widget. > > thank you > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=7a1299e262185c88e376cd20f10620839ad6c37d sorry, pressed reply to the wrong comment. this is the patch for "Bug fix - pass the correct flags into DrawThemeTextEx from Label widget."
Hi Raymond, the code is definitely going the right direction, but I got this exception here: Exception in thread "main" java.lang.StringIndexOutOfBoundsException: String index out of range: 20 at java.lang.String.charAt(Unknown Source) at org.eclipse.swt.widgets.Link.wmBufferedPaint(Link.java:1112) at org.eclipse.swt.widgets.Control.WM_PAINT(Control.java:5120) at org.eclipse.swt.widgets.Link.WM_PAINT(Link.java:1022) at org.eclipse.swt.widgets.Control.windowProc(Control.java:4615) at org.eclipse.swt.widgets.Display.windowProc(Display.java:5009) at org.eclipse.swt.internal.win32.OS.UpdateWindow(Native Method) at org.eclipse.swt.widgets.Decorations.setVisible(Decorations.java:1435) at org.eclipse.swt.widgets.Shell.setVisible(Shell.java:1927) at org.eclipse.swt.widgets.Shell.open(Shell.java:1251) at tests.AeroLabel.main(AeroLabel.java:63) here the code snippet: Link link; link = new Link(shell, trim); link.setText("link <a>trim</a> other <a>link</a>");
can this line color = (style.foreground.getBlue() << 16) + (style.foreground.getGreen() << 8) + style.foreground.getRed(); be changed to color = style.foreground.handle; ?
I pushed the Link implementation anyway (since it is in the right shape), http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK&id=99429d6dcd838b8ee5cda3dbd63fdc77a1df49e1 please fix the bug I found in comment 131, thank you
Created attachment 209478 [details] Fix StringIndexOutOfBoundsException in Link Widget Thanks Felipe - your requests in comments 131 and 132 are addressed by this patch.
(In reply to comment #134) > Created attachment 209478 [details] > Fix StringIndexOutOfBoundsException in Link Widget > > Thanks Felipe - your requests in comments 131 and 132 are addressed by this > patch. Thanks http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK Now that I can run it I see that the underline color is wrong.. should be a easy fix.
(In reply to comment #135) > > Now that I can run it I see that the underline color is wrong.. should be a > easy fix. maybe not, I create to create a simple pen with the right color but it is not the same color as the text (since the text has glow/alpha). Anyway DrawThemeTextEx () can draw underline text ? maybe in the worse case we can create a new logical font with the underline bit set...
Created attachment 209489 [details] Bug Fix - Draw underlining of Link using logical fonts Good catch Felipe - this version uses your suggested logical font approach and does the trick. Give it a shot.
(In reply to comment #137) > Good catch Felipe - this version uses your suggested logical font approach and > does the trick. Give it a shot. Thank: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?h=AERO_WORK Raymond, what is the next step ?
I'm closing bugs Bug329292 Bug329293 Bug329299 as they were discussed and fixed in this bug. is that okay ?
as far as I know there is still a few problems here: SWT.Paint event and custom widgets: I don't think TRIM_FILL are getting Paint event. Even if they did, there is no support for them to draw text correctly. For example, try add an CLabel to the trim. Note that if we had support to draw "glow" text in GC and TextLayout fixing Link would have been trivial. Some trim controls with image do not draw correctly (depends on Image). Some trim controls to border looks ugly (Combo). I see pixel corruption at times when traversing over several trim controls. setData() passing the secret word is not API that we advise having. Are this limitation that you are okay with ? Raymond, as per Eclipse policy I need to declare that: "I wrote all this code and have the rights to contribute it to Eclipse under the eclipse.org web site terms of use." See http://wiki.eclipse.org/Development_Resources/Handling_Git_Contributions
Felipe, thanks for your help on this. Raymond is unavailable but will respond as soon as he's back.
Hi Felipe, Thanks for your feedback, and yes there are some known limitations, and indeed our coverage of the SWT widget family for Aero/Glass support is limited to the set of controls that we had established internally at the start - namely: Label, Button, ProgressBar, Link, Text, Combo and Toolbar/Item. Others like Composite, Canvas and Scrollable derive their support by inheriting off of Control and the work we've done there in the base class. > SWT.Paint event and custom widgets: > > I don't think TRIM_FILL are getting Paint event. > Even if they did, there is no support for them to draw text correctly. > For example, try add an CLabel to the trim. > Note that if we had support to draw "glow" text in GC and TextLayout fixing > Link would have been trivial. Unfortunately, custom widgets were not on our list as enumerated above. I think this should be pretty straightforward though, given the work we've established on Label. > > Some trim controls with image do not draw correctly (depends on Image). Yes, certain types of images (those that don't support alpha channel) render poorly. This is a known limitation. > > Some trim controls to border looks ugly (Combo). Yes, very much agree. The implementation for some of these widgets, like Combo, have been to ask them to draw themselves opaquely into device context that is Glass aware. This can sometimes lead to an obvious clashing of styles as the native drawing is not as elegant as those that are specifically Glass aware. > > I see pixel corruption at times when traversing over several trim controls. I've seen those sometimes too, and would require further investigation to see where the corruption is coming from. > > setData() passing the secret word is not API that we advise having. Yes, we are agreed on that. We'd have to come up with a different design I suppose to get around this shortcoming. > > Are this limitation that you are okay with ? As a first pass, I think what we have done as a proof of concept definitely shows what is possible with a Glass/Aero integration into the Eclipse framework. If we were to target Glass/Aero usage to what we typically see in Windows apps, which is usually in the top and in the toolbar region, the current set of controls can be used quite effectively to add that splash of UI to make applications seem more modern. > Raymond, as per Eclipse policy I need to declare that: > "I wrote all this code and have the rights to contribute it to Eclipse under > the > eclipse.org web site terms of use." > See http://wiki.eclipse.org/Development_Resources/Handling_Git_Contributions And yes, I Raymond Lam, wrote all this code and have the rights to contribute it to Eclipse under the eclipse.org web site terms of use.
Hi all, may I ask what the current status is? I had the impression that everything is on its way when I read this thread. When I take a look into the git repo then the AERO_WORK branch was kept up-to-date only until 2012-02-01... That's a pity. Is there any blocker or major decision keeping you from preserving this piece of work? It looks very promising to me. Thx in advance
Vista is not supported anymore, please reopen if that feature is still supported by Win10 and still desired to be supported by SWT.
