Community
Participate
Working Groups
Build ID: I20080207-1530 Steps To Reproduce: This is related to bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=219432 Bug 219432 improved image loading performance by using SWTs native OS image loading code. ImageDescriptor.createFromURL(imgURL) should sniff the URL to exploit the improvement in FileImageDescriptor. Something along the lines of: URL imgURL = new URL("big.png"); URL local = FileLocator.toFileURL(imgURL); ImageDescriptor imgDesc; if(local.getProtocol().equals("file")) imgDesc = ImageDescriptor.createFromFile(null, local.toExternalForm().substring(6)); // "file://".length() else imgDesc = ImageDescriptor.createFromURL(FileLocator.resolve(imgURL)); themeImg = imgDesc.createImage(false); More information:
Created attachment 94036 [details] patch Here is an OSGI friendly patch. I am currently not happy with this as we are trying to open files first and then falling through to the URL stream loading. I would prefer if there was a better way to introspect the URL. See the example snippet which loads from a file and an http URL.
I would like some advice from John and Szymon as they are the resource guys.
This doesn't seem relate to resources, but the only advice I have is to introspect the protocol rather than directly trying to interpret URL.getPath(): String path = null; if ("file".equalsIgnoreCase(url.getProtocol())) { path = url.getPath(); if (path != null) { .. use file loading } else { .. use URL stream loading ... }
John does this handle the OSGi case where the protocol is bundle. i.e. should I do this check only if there is no OSGi and if there is can I just rely on FileLocator?
> should I do this check only if there is no OSGi and if there is can I just rely on FileLocator? I would do this check both in the non-OSGi case, and on the URL returned from FileLocator#toFileURL(). Note toFileURL says "@return the converted file URL or the original URL passed in if it is not recognized by this converter", so you can't assume that it will always be a file: URL you get back.
Fixed with John's suggestions in build >20080403
Todd, I back-ported your patch to Eclipse 3.2.2 and in the process noticed that it's not actually making use of the faster SWT image loading due to a path issue. In FileImageDescriptor and URLImageDescriptor getPath() returns a path that looks like this on Windows. /c:/temp/a.png. Image(Device,String) doesn't like that path, no exceptions of course, it just takes the slow path instead. When I trim the leading slash i.e. c:/temp/a.png the fast path is chosen and I see ~50% improvement to image loading. I'm using locatedURL.getPath().substring(1); seem reasonable?
Chris if you could attach a new patch that would be really useful.
Created attachment 96248 [details] Strip leading slash Seems safer to only strip the leading slash as opposed to always doing a substring(1). I've only tried this on Windows. I'm not sure if the other platforms have a similar SWT API to even call.
The trim is only correct on Windows - a leading slash is correct on a Unix based platform so stripping the / is not the right answer. However you are right that we need to remove it on Windows. Szymon or John what is recommended approach to doing this?
Created attachment 96273 [details] Updated patch This patch uses the same idea but processes using the Path object so as to get correct behaviour on all platforms. I have also added a failsafe for FileImageDescriptor that calls the slower getStream SWT API when we fail as I did for URLImageDescriptor. This should solve the exception we are seeing in Bug 225182. Chris I would appreciate it if you could check this out.
Created attachment 96277 [details] Small refinement Due to one missed case. Released to HEAD for build >20080416 but I will leave this open pending confirmation from Chris.
Tod - I grabbed head, applied to my environment and and looks/works great.
When this fix is made, we are also interest in back porting to Eclipse 3.2.2
Tod, how hard would it be to build a 3.2.2 patch for this?
It is one class so just loading the 3.4 version into 3.3.2 and creating a patch should be all you need.
Marking FIXED
Created attachment 97067 [details] R 3.3.2 version Here is a 3.3.2 version. it uses the pre 3.4 error reporting as well.
Hi Tod, we need to back port to Eclipse 3.2.2 release, just want to make sure your patch is for 3.2.2 or 3.3.2?
That is a 3.3.2 patch - you would likely need a different one for 3.3.2 but it would have the same code content I would think.
Hi Tod, thanks for quick response, could you please provide a patch for 3.2.2?
The problem with using 3.2.2 is that getFilePath() references the JFaceActivator which was not added until 3.3. Here is the code. John can you think of an alternative way to determine if OSGi is present? Here is the code in question. if (JFaceActivator.getBundleContext() == null) { if (FILE_PROTOCOL.equalsIgnoreCase(url.getProtocol())) return new Path(url.getFile()).toOSString(); return null; } URL locatedURL = FileLocator.toFileURL(url); if (FILE_PROTOCOL.equalsIgnoreCase(locatedURL.getProtocol())) return new Path(locatedURL.getPath()).toOSString(); return null;
Verified in I20080428-0100
