Re: RFR: 8258979: The image didn't show correctly with GTK LAF [v5]
On Thu, 11 Jan 2024 07:07:10 GMT, Prasanta Sadhukhan
wrote:
>> Abhishek Kumar has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Review comment fix and move file outside folder
>
> src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTreeUI.java line
> 161:
>
>> 159:
>> 160: expandedIconWrapper = new IconWrapper(expandedIcon);
>> 161: collapsedIconWrapper = new IconWrapper(collapsedIcon);
>
> It seems `updateStyle` is called for all `propertyChange` event so it can
> cause memory leak by doing the class instantiation every time it is called..
> Probably better place to instantiate this objects is in `installDefaults`
> after calling `updateStyle`
Updated.
> src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTreeUI.java line
> 809:
>
>> 807: }
>> 808: else {
>> 809: SynthGraphicsUtils.paintIcon(iconType, context, g, x,
>> y, w, h);
>
> Guess it can be further optimized in all these methods
>
> if (context == null) {
> context = getContext(tree);
> }
> SynthGraphicsUtils.paintIcon(iconType, context, g, x, y, w, h);
Updated
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17294#discussion_r1449015436
PR Review Comment: https://git.openjdk.org/jdk/pull/17294#discussion_r1449015571
Re: RFR: 8258979: The image didn't show correctly with GTK LAF [v5]
On Thu, 11 Jan 2024 05:21:51 GMT, Abhishek Kumar wrote:
>> The collapsed icon for JTree is not painted using `Icon.paintIcon(Component
>> c, Graphics g, int x, int y)` in GTK LAF. The collapsed icon is returned
>> from BasicTreeUI class which doesn't contain any icon image whereas the
>> expanded icon is returned from SynthTreeUI class and expanded icon is
>> rendered correctly.
>> The proposed fix is to return collapsed icon as an object of collapsed icon
>> wrapper which implements synthIcon and is similar to the expanded icon
>> implementation.
>>
>> Test mentioned in JBS is an applet based which is converted to main based
>> now and extended for all installed LAFs on the system.
>>
>> No regression caused with the fix, link is attached in JBS .
>
> Abhishek Kumar has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Review comment fix and move file outside folder
src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTreeUI.java line 161:
> 159:
> 160: expandedIconWrapper = new IconWrapper(expandedIcon);
> 161: collapsedIconWrapper = new IconWrapper(collapsedIcon);
It seems `updateStyle` is called for all `propertyChange` event so it can cause
memory leak by doing the class instantiation every time it is called..
Probably better place to instantiate this objects is in `installDefaults`
after calling `updateStyle`
src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTreeUI.java line 809:
> 807: }
> 808: else {
> 809: SynthGraphicsUtils.paintIcon(iconType, context, g, x, y,
> w, h);
Guess it can be further optimized in all these methods
if (context == null) {
context = getContext(tree);
}
SynthGraphicsUtils.paintIcon(iconType, context, g, x, y, w, h);
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17294#discussion_r1448379415
PR Review Comment: https://git.openjdk.org/jdk/pull/17294#discussion_r1448380657
Re: RFR: 8258979: The image didn't show correctly with GTK LAF [v5]
> The collapsed icon for JTree is not painted using `Icon.paintIcon(Component > c, Graphics g, int x, int y)` in GTK LAF. The collapsed icon is returned from > BasicTreeUI class which doesn't contain any icon image whereas the expanded > icon is returned from SynthTreeUI class and expanded icon is rendered > correctly. > The proposed fix is to return collapsed icon as an object of collapsed icon > wrapper which implements synthIcon and is similar to the expanded icon > implementation. > > Test mentioned in JBS is an applet based which is converted to main based now > and extended for all installed LAFs on the system. > > No regression caused with the fix, link is attached in JBS . Abhishek Kumar has updated the pull request incrementally with one additional commit since the last revision: Review comment fix and move file outside folder - Changes: - all: https://git.openjdk.org/jdk/pull/17294/files - new: https://git.openjdk.org/jdk/pull/17294/files/2fa05f9f..17d9bdc2 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=17294=04 - incr: https://webrevs.openjdk.org/?repo=jdk=17294=03-04 Stats: 165 lines in 2 files changed: 5 ins; 152 del; 8 mod Patch: https://git.openjdk.org/jdk/pull/17294.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17294/head:pull/17294 PR: https://git.openjdk.org/jdk/pull/17294
