Re: JDK-8130738 TextFlow's tab width is static

2019-09-23 Thread Scott Palmer
My current work is here 
https://github.com/javafxports/openjdk-jfx/compare/develop...swpalmer:jdk-8130738?expand=1
 


While writing a unit test I realized that the StubToolkit isn’t really running 
the Prism layout code, so I’m not sure how that gets tested.  I made it work 
with StubTextLayout for now.

Regards,

Scott


> On Sep 20, 2019, at 12:57 PM, Scott Palmer  wrote:
> 
> Thanks Kevin.
> 
> My current implementation appears to be working for TextFlow and Text, with 
> the TextFlow overriding the tabWidth of the child Text nodes.  This seems to 
> work out naturally from the way TextFlow overrides the TextLayout instance 
> used by the Text node.
> 
> If there are tricky corner-cases that I’m missing, I guess figuring out all 
> the cases it will need to handle is part of this discussion.  It didn’t seem 
> to be that challenging so far, so perhaps I am missing something (hopefully 
> not).  I wrote a small test app to visually see that things were working as I 
> expected.  I have not yet written the unit tests.
> 
> Cheers,
> 
> Scott
> 
>> On Sep 20, 2019, at 10:58 AM, Kevin Rushforth  
>> wrote:
>> 
>> Hi Scott,
>> 
>> I'm sure Phil will have more comments on this. While the API seems simple 
>> enough on the surface, I suspect that this will be a challenging feature to 
>> implement correctly for all of the cases it will need to handle. It would 
>> need careful review and testing as well. My only comment on the API itself 
>> is that if we do accept this feature, it should probably go on both Text and 
>> TextFlow, and be one of the attributes of Text that is ignored / overridden 
>> when a Text node is in a TextFlow.
>> 
>> -- Kevin
>> 
>> 
>> On 9/18/2019 6:14 PM, Scott Palmer wrote:
>>> I would like to implement this feature, being able to adjust the tab size 
>>> in a TextFlow or Text node (JDK-8130738 
>>> ).  It involves new 
>>> public API, so I want to start a discussion about it here.  (My motivation 
>>> is that RichTextFX suggests an entirely unacceptable workaround of 
>>> substituting actual spaces when the tab character is typed and cites the 
>>> lack of this API.)
>>> 
>>> I’ve already jumped the gun and taken a crack at an implementation.  It is 
>>> currently incomplete as I was just poking around to see if it was going to 
>>> be easy enough to not take up too much of my time.  It boils down to:
>>> TextFlow and Text get a new property for tab width, an integer representing 
>>> the number of spaces taken by a tab. (The value is only used to initialize 
>>> the tab width for the TextLayout when needed.)
>>> TextLayout interface gets a new method:  boolean setTabWidth(int spaces)
>>> TextLayout gets a new constant: DEFAULT_TAB_WIDTH = 8;
>>> PrismTextLayout implements the new setTabWidth API.
>>> 
>>> I’m not sure that the Text node needs this new property.  I figured it 
>>> would be rarely used on that class, so I had implemented it via an added 
>>> property in the private TextAttributes class.  Maybe it isn’t needed at all.
>>> 
>>> What’s the next step?
>>> 
>>> Regards,
>>> 
>>> Scott
>> 
> 



Re: Use of Hashtable in Prism

2019-09-23 Thread Kevin Rushforth
No good reason that I can think of. This seems like something that could 
be cleaned up if someone wanted to file an issue and contribute a fix.


-- Kevin


On 9/23/2019 10:18 AM, Scott Palmer wrote:

I just noticed that NetBeans is warning me about use of an “obsolete 
collection” in PrismTextLayout.java.

Is there a reason this isn’t using HashMap or ConcurrentHashMap ?

Scott





Re: Use of Hashtable in Prism

2019-09-23 Thread Philip Race

I am not sure but mutation of the Hashtable only happens from inside a
block synchronized on CACHE_SIZE_LOCK.

Perhaps this somehow plays into it .. in some way I can't see.
But I am told that uncontended locks are quite cheap these days.

-phil

On 9/23/19, 10:18 AM, Scott Palmer wrote:

I just noticed that NetBeans is warning me about use of an “obsolete 
collection” in PrismTextLayout.java.

Is there a reason this isn’t using HashMap or ConcurrentHashMap ?

Scott



Use of Hashtable in Prism

2019-09-23 Thread Scott Palmer
I just noticed that NetBeans is warning me about use of an “obsolete 
collection” in PrismTextLayout.java.

Is there a reason this isn’t using HashMap or ConcurrentHashMap ?

Scott