[squeak-dev] wait2ms (The Trunk: ST80-dtl.141.mcz)
David T. Lewis
lewis at mail.msen.com
Sat Feb 9 20:29:43 UTC 2013
Bob,
I tried reverting zapSelectionWithCompositionWith: to its original
implementation, and making the change that you suggest to readKeyboard.
It works exactly as you describe, including when I type rapidly in an
image with the wait2ms delay.
I will make this update to Squeak trunk. To summarize the changes
we will have at that point:
1) The wait2ms is removed
2) ParagraphEditor>>readKeyboard will always process one character at a time
3) No change to zapSelectionWithCompositionWith:
Thanks very much for your help on this!
Dave
On Sat, Feb 09, 2013 at 02:57:32PM -0500, Bob Arning wrote:
>
> On 2/9/13 1:48 PM, David T. Lewis wrote:
> >Hi Bob, thanks for this.
> >
> >I am hopelessly out of my depth on multilingual processing and multibyte
> >characters, but I'm sure others on the list will know: Are there cases
> >(e.g. in Japan or Korea) where keyboards provide multi-byte inputs, either
> >directly or through X11? If so, the "sensor keyboardPressed whileTrue:"
> >may be necessary in order to process those key entries.
> Remember that there is another "[sensor keyboardPressed] whileTrue: "
> higher up that encompases this, so I don't think this is something to
> worry about.
> >Otherwise, I
> >suspect you are right that it is easier to just process one character at
> >a time. I note also that the "whileTrue:" approach dates back to at least
> >2002, well before introduction of multilingual support in Squeak. I don't
> >know if that's significant, or if that's just the way it happened to have
> >been done.
> It was that way in 1998. My guess is that if multiple input characters
> were available, it made some sense efficiency-wise to insert them into
> the text all at once rather than piecemeal. Back the people typed just
> as fast as today, but the processors were much slower.
> >
> >Just by way of explanation, here is how I found and verified the change
> >to zapSelectionWithCompositionWith: in ST80-dtl.141.mcz:
> >
> >1) In the original method, I forced the method to always execute the
> >multi-character path through the code, even if the input string was one
> >character long (the normal case). So I changed from this:
> >
> > ((aString isEmpty or: [(beforeChar := self charBefore) isNil]) or: [
> > aString size = 1 and: [(Unicode isComposition: aString first) not]])
> > ifTrue: [
> > ^ self zapSelectionWith: (Text string: aString emphasis:
> > emphasisHere)].
> >
> >to this:
> >
> > ((aString isEmpty or: [(beforeChar := self charBefore) isNil]) or: [
> > aString size = 1 and: [(Unicode isComposition: aString first) not]])
> > ifTrue: [
> > "^ self zapSelectionWith: (Text string: aString emphasis:
> > emphasisHere)" ].
> >
> >Result: All characters typed into an MVC workspace resulted in the cursor
> >positioned incorrectly to the left of the current character.
> >
> >2) I changed the method to reposition the cursor correctly, leaving the
> >above change in place to continue forcing execution of the multi-byte code
> >path. This fixed the cursor positioning problem in the MVC workspace.
> >
> >3) I removed the change from step 1 so that single character input would
> >be handled through the normal path, and verified that the MVC workspace
> >still worked.
> >
> >4) I filed the updated method into another image that still has the wait2ms
> >delays, and I ran that image on a Linux interpreter VM (with "2 ms" delays
> >that might be more like 15 ms). Result: Cursor positioning now worked
> >correctly
> >in the MVC workspace, even though keyboard entry was slow.
> I put your change in an otherwise unchanged 4.4 and mashed keys very
> quickly. Everything got entered, but the cursor got farther and farther
> behind the end of the text. This is on a Mac where 2ms seems to be 2ms.
> I then changed wait2ms to wait 20ms and things got worse - it took long
> pauses for characters to appear and when they did, the cursor did not
> keep pace with the end of the text. What might be interesting to know
> how often (if ever) your tests actually had more that one byte to
> process in the zap methods. I think a single character works fine either
> way, multiple characters work fine in the original zap.. but multiple
> characters in the new zap are the problem.
> >
> >Conclusion: The patch in ST80-dtl.141.mcz does address a real bug in cursor
> >positioning that occured only in cases where sluggish key entry resulted
> >in multi-byte strings (presumably two characters) being handled in the
> >zapSelectionWithCompositionWith: method.
> >
> >The problem that you identified with your "z123" test does not seem to be
> >occurring in practice,
> But it did and that's what got me started here - I was trying to see
> sluggish input in MVC, so I mashed keys as fast as I could and they all
> made it to the screen, just not always at the end of the text.
> >but it certainly looks as though it *could* occur.
> >I tested only by typing into a workspace from my US ascii keyboard, so I
> >certainly would not rule out the possibility of further problems.
> >
> >I do not know if it is safe to make the change you suggest to process one
> >character at a time. It looks to me like it would be good to do that, but
> >I don't know if it might cause problems for non-US keyboards or locales.
> >
> >If it was safe to make that change, then the entire bit of code for
> >handling
> >multi-byte strings in zapSelectionWithCompositionWith: would become
> >irrelevant,
> >and we could just get rid of it.
> I don't think so. That code has a purpose - the UnicodeCompositionStream
> is used to combined multiple characters (like a letter followed by a
> separate accent character) into a single character. It is basically
> saying: if there are 1 or more characters in the typeahead and the first
> is one of these combining characters (accent) and there is a character
> in the text before the insertion point, then back up 1 space so we can
> process that character and the accent together. I suspect it works fine
> when there is just once character in the typeahead and that character is
> an accent. I don't think it works well when there are more than one
> character in the input. Hence my suggestion to do it a char at a time.
>
> Cheers,
> Bob
> >
> >Hopefully someone with experience in this area can help?
> >
> >Dave
> >
> >
> >On Sat, Feb 09, 2013 at 06:52:43AM -0500, Bob Arning wrote:
> >>I fear that doesn't even work with ordinary USASCII input. I tweaked
> >>readKeyboard to simulate additional characters arriving in close
> >>succession:
> >>
> >> self hasSelection ifTrue: "save highlighted characters"
> >> [UndoSelection := self selection].
> >>*((model isKindOf: Workspace) and: [char = $z]) ifTrue: [typeAhead
> >>nextPutAll: '123'].*
> >>
> >> self zapSelectionWithCompositionWith: typeAhead contents.
> >>
> >>and what I get by typing some z's in a Workspace is:
> >>
> >>z123zzzzzz123123123123123123
> >>
> >>I think the safest way to guard against multiple characters arriving in
> >>quick succession is to simply process each one completely before
> >>proceeding to the next. This, after all, is the likeliest case by far
> >>for modern computers and ordinary humans typing:
> >>
> >> self deselect.
> >>*sensor keyboardPressed ifTrue: "was a whileTrue:"*
> >> [char := sensor keyboardPeek.
> >> (self dispatchOnCharacter: char with: typeAhead) ifTrue:
> >> [self doneTyping.
> >> self setEmphasisHere.
> >> ^self selectAndScroll; updateMarker].
> >> self openTypeIn].
> >>
> >>Cheers,
> >>Bob
> >>
> >>On 2/9/13 1:01 AM, commits at source.squeak.org wrote:
> >>>aText := Text string: newString emphasis: emphasisHere.
> >>>- self markBlock < self pointBlock
> >>>- ifTrue: [self setMark: self markBlock stringIndex - 1]
> >>>- ifFalse: [self setPoint: self pointBlock stringIndex - 1].
> >>>-
> >>> wasComposition := true.
> >>>+ self markBlock < self pointBlock
> >>>+ ifTrue: [ self setMark: self markBlock stringIndex - 1.
> >>>+ self zapSelectionWith: aText.
> >>>+ self setMark: self markBlock stringIndex + 1]
> >>>+ ifFalse: [ self setPoint: self pointBlock stringIndex - 1.
> >>>+ self zapSelectionWith: aText.
> >>>+ self setPoint: self pointBlock stringIndex +
> >>>1]
> >>>- self zapSelectionWith: aText.
> >
> >
>
More information about the Squeak-dev
mailing list
|