-
Notifications
You must be signed in to change notification settings - Fork 182
GC#DrawImage(image, destX, destY, destWidth, destHeight) now tries to load images at the requested destination size before drawing #2526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
01766e0
to
fa4a774
Compare
8cf1bfb
to
063d70b
Compare
063d70b
to
a9b9404
Compare
82c03f5
to
daf247e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I scanned the PR and commented on some obvious or essential points I came across.
bundles/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/internal/image/FileFormat.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/GC.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
e7af9a1
to
d25c48a
Compare
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
0e9ab55
to
d3575ff
Compare
b59aa2d
to
582685a
Compare
582685a
to
67d9379
Compare
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
67d9379
to
38f10a0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the latest cleanups. They made the code much easier to understand.
Reading the code again I see that, if I am not mistaken, it breaks the existing (implicit) contract for the cache image data, such that it would like lead to a regression. That's why I request changes to avoid that we break existing behavior,
In addition, all the changes related to creating a temporary handle based on zoom for executing a drawing operation on look far more complicated than they need to be to me. I wrote a longer comment trying to explain what in my opinion can/should be done to improve that.
...les/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageDataAtSizeProvider.java
Outdated
Show resolved
Hide resolved
...les/org.eclipse.swt/Eclipse SWT/common/org/eclipse/swt/graphics/ImageDataAtSizeProvider.java
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/graphics/Image.java
Outdated
Show resolved
Hide resolved
30de03f
to
fe7048d
Compare
3875d39
to
d07939a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me and I didn't find any regression testing it with the Runtime or the CustomControlExample that already use the new GC API that triggers the changed logic
d07939a
to
db2d773
Compare
8c1661a
to
627d661
Compare
…en possible. The DrawImage API with just destination coordinates now tries to load images at the requested size. If the requested size is not available, it falls back to the closest available size by calculating the appropriate zoom. Images are loadable at the target size if the image is created from an SVG stream or filename, or The image is created with an ImageDataProvider that is ImageDataAtSizeProvider. This commit also introduces the ImageDataAtSizeProvider interface to support dynamic sizing. Co-authored-by: Michael Bangas <michael.bangas@vector.com>
627d661
to
b238dec
Compare
The DrawImage API with just destination coordinates now tries to load images at the requested size.
If the requested size is not available, it falls back to the closest available size by calculating the appropriate zoom.
Images are loadable at the target size if the image is created from an SVG stream or filename, or The image is created with an ImageDataProvider that is ImageDataAtSizeProvider.
This commit also introduces the ImageDataAtSizeProvider interface to support dynamic sizing.
With this changes we need to test the following
Steps to reproduce:
snippet 1 to check drawing svgs at custom sizes (click to show snippet)
**Step 1**: Copy the eclipse.svg to same path where the snippets are and run the below snippet. **Step 2**: Run the snippetpackage org.eclipse.swt.snippets;
before
after
**snippet 2** Drawing images with ImageDataAtSizeProvider
package org.eclipse.swt.snippets;
This depends on #2514 and #2509, Only last commit is relevant to this PR