Re: RFR: 8330559: Trailing space not rendering correctly in TextFlow in RTL mode
On Sat, 1 Jun 2024 04:53:48 GMT, John Hendrikx wrote: >> modules/javafx.graphics/src/main/java/com/sun/javafx/text/TextRun.java line >> 332: >> >>> 330: return x; >>> 331: } >>> 332: int prevIdx = (glyphIndex - 1)<<1; >> >> would it be possible to add a comment explaining why this code is doing what >> it's doing? specifically, the reason for the while() loop and Math.abs(). >> and perhaps mention that this happens on mac only. > > I think if this is specific to mac, and doesn't occur on other platforms that > the supplied `positions` may be incorrect by the underlying `*GlyphLayout` > code. Under Windows it will likely use the `DWGlyphLayout` to supply > `positions`. On Mac this will be `CTGlyphLayout`. > > The more prudent course of action than seems to be to fix what is supplied by > `CTGlyphLayout` instead of working around it in `TextRun`. > > I haven't checked it that extensively yet, but I think `positions` is not > supposed to contain negative `x` values at all when not in compact mode. Yes the issue is specific to Mac only. Windows uses `DWGlyphLayout` for positions as you pointed out. Since the position obtained from the underlying platform itself is not correct Mac, looked like fixing it in either `TextRun` or in the native side does not make huge difference so went with this approach. However I will explore how we can fix it on the native side. If the `positions` is not supposed to contain negative value, it is an issue from Apple not in JavaFX. In this case should we actually fix the issue in JavaFX, as it becomes a workaround rather than a fix regardless of where we fix it. - PR Review Comment: https://git.openjdk.org/jfx/pull/1468#discussion_r1625369735
Re: RFR: 8330559: Trailing space not rendering correctly in TextFlow in RTL mode
On Fri, 31 May 2024 17:21:56 GMT, Andy Goryachev wrote: > Seems to work with the example in the ticket and the TextArea in the Monkey > Tester. No ill effects on Windows. (Did not test it on Linux) > I tested the issue on Linux. It is not reproducible. So this is specific to Mac only. > Could you come up with a unit test? I couldn't find a way to write unit test for this. I will explore more to find out if it is possible. Please let me know if you have any idea regarding this. - PR Comment: https://git.openjdk.org/jfx/pull/1468#issuecomment-2146638068
Re: RFR: 8332895: Support interpolation for backgrounds and borders
On Mon, 3 Jun 2024 09:48:40 GMT, Florian Kirmaier wrote:
> Can you elaborate on this? Are changes in the Region.background still trigger
> change events? If not, is there a mechanism to get them? Like
> Transform.onTransformChanged? Are the previous immutable objects like
> Background and Border now mutable?
I'm not entirely sure what you mean here, but nothing how backgrounds and
borders work has changed. Both are still deeply immutable, what's new is that
they implement `Interpolatable`.
For example, if you want to change the background color of a region, you can't
reassign `BackgroundFill.fill`. Instead, you will have to create a new
`Background` instance that contains a new `BackgroundFill`, which contains the
new color. This is what CSS does when it applies a new background color.
This PR allows the CSS system to also create intermediate backgrounds that
represent the transition from one color to the next.
The `interpolate()` method may or may not return a new instance of the
interpolatable object. For example, consider the current version of
`Color.interpolate()`, which returns `this` for t<=0 and `endValue` for t>=1
(in other words, a new instance is not returned in all cases):
@Override
public Color interpolate(Color endValue, double t) {
if (t <= 0.0) return this;
if (t >= 1.0) return endValue;
...
}
However, the current specification of `Interpolatable.interpolate()` is a bit
unclear on that:
/*
* The function calculates an interpolated value along the fraction
* {@code t} between {@code 0.0} and {@code 1.0}. When {@code t} = 1.0,
* {@code endVal} is returned.
*/
The updated specification is clearer:
/**
* Returns an intermediate value between the value of this {@code
Interpolatable} and the specified
* {@code endValue} using the linear interpolation factor {@code t},
ranging from 0 (inclusive)
* to 1 (inclusive).
*
* The returned value may not be a new instance; an implementation might
also return one of the
* two existing instances if the intermediate value would be equal to one
of the existing values.
* However, this is an optimization and applications should not assume any
particular identity
* of the returned value.
*
* An implementation is not required to reject interpolation factors less
than 0 or larger than 1,
* but this specification gives no meaning to values returned outside of
this range. For example,
* an implementation might clamp the interpolation factor to [0..1], or it
might continue the linear
* interpolation outside of this range.
*/
-
PR Comment: https://git.openjdk.org/jfx/pull/1471#issuecomment-2145035539
Re: RFR: 8332895: Support interpolation for backgrounds and borders
On Sun, 2 Jun 2024 18:50:20 GMT, Michael Strauß wrote: > This PR completes the CSS Transitions story (see #870) by adding > interpolation support for backgrounds and borders, making them targetable by > transitions. > > More specifically, the following types will now implement `Interpolatable`. > > - `Insets` > - `Background` > - `BackgroundFill` > - `BackgroundImage` > - `BackgroundPosition` > - `BackgroundSize` > - `Border` > - `BorderImage` > - `BorderStroke` > - `BorderWidths` > - `CornerRadii` > - `ImagePattern` > - `LinearGradient` > - `RadialGradient` > - `Stop` > > Note that this PR also changes the specification of `Interpolatable` to make > users aware that they shouldn't assume any particular identity of the object > returned from the `interpolate()` method. This allows the implementation to > re-use objects and reduce the number of object allocations. First, Thank you for working on these transitions! > Note that this PR also changes the specification of Interpolatable to make > users aware that they shouldn't assume any particular identity of the object > returned from the interpolate() method. This allows the implementation to > re-use objects and reduce the number of object allocations. Can you elaborate on this? Are changes in the Region.background still trigger change events? If not, is there a mechanism to get them? Like Transform.onTransformChanged? Are the previous immutable objects like Background and Border now mutable? - PR Comment: https://git.openjdk.org/jfx/pull/1471#issuecomment-2144762597
