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

Brace highlighting causes neighbor character's style to "flicker" #396

Closed
ervumlens opened this Issue Jul 5, 2015 · 12 comments

Comments

Projects
None yet
4 participants
@ervumlens

ervumlens commented Jul 5, 2015

Problem
Typing next to a brace-highlighted character (e.g., before a ]) causes the typed character to adopt the brace highlighting style briefly, then switch to normal. This looks like a flicker at normal typing speed. This is most obvious on a dark background with bold white brace highlighting, which is essentially what I'm using.

This happens whether or not soft characters are enabled.

Komodo Edit, version 9.1.0, build 15798, platform linux-x86_64.
Built on Sat May 30 05:22:08 2015.

Steps to Reproduce

  1. Open a file.
  2. Type [].
  3. Put the cursor between the brackets.
  4. Start typing.

Expected
The typed character appears in its normal style. The braces continue to appear highlighted.

Actual
The typed character appears with the highlighted style. The neighboring brace appears with its unhighlighted style. A moment later the styles are corrected.

Tapping 3:
image

A moment later this (correctly) turns into:
image

Workaround
No effective one that I can find. Brace highlighting is always on, so there is always a highlight style active.

P.S.
If your computer is too fast to exhibit the problem, please send it to me and I will promptly close this issue. 😉 Or try setting your bracehighlight style to a ridiculously large font size, like so:

image

Or hold the key down rather than tapping it:

image

@Naatan

This comment has been minimized.

Member

Naatan commented Jul 6, 2015

My guess is this is a scintilla "issue" that's by design. @mitchell-as can you comment?

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Jul 6, 2015

Correct. Scintilla has a character buffer and a style buffer. The character buffer holds buffer text and the style buffer holds styles to draw characters in. They need to be resynchronized as text is inserted and removed, and it is very likely you are capturing Scintilla in the middle of a resynchronization attempt. I doubt there's anything we can do here.

@ervumlens

This comment has been minimized.

ervumlens commented Jul 6, 2015

Ok, thanks for digging into it. I was unable to reproduce the problem in SciTE or CodeBlocks (which I think still uses Scintilla), so I thought it might be a Komodo thing. If I find anything of interest, I'll post it here.

@ervumlens

This comment has been minimized.

ervumlens commented Jul 6, 2015

The style sync'ing in this case occurs in views-buffer-base::onUpdateUI. The actual style updating occurs behind a setTimeout call. This accounts for the delay in updating Scintilla. Making the styles update directly in onUpdateUI was enough to resolve the flickering.

This would also explain why I can't reproduce the issue in SciTE or other Scintilla-based editors.

The update occurs in a setTimeout because "often painting during onUpdateUI doesn't work correctly" but "if we set it in a timeout, it works".

The changes were made in 2009, and it's not immediately clear to me what didn't work correctly then or whether the timeout is the most appropriate solution to that problem.

Concerning the flicker issue in particular: reducing the timeout delay to 5ms (from 50ms) made a visually significant reduction to the flicker. Even in the extreme case of holding down the key to force a repeat, the shorter delay made the problem surprisingly less noticeable.

@ervumlens

This comment has been minimized.

ervumlens commented Jul 6, 2015

I wrote down a few more details on my fork's issue, in case you're morbidly curious.

@Naatan

This comment has been minimized.

Member

Naatan commented Jul 6, 2015

Thanks for digging into that @ervumlens ! I just spoke about it with Mitchell and he thinks its too risky to fix this for the upcoming build, as Komodo does a lot of things with Scintilla under the hood. We'd need to test over a longer period of time whether removing that delay causes any nasty issues that may not be immediately obvious.

So we'll definitely look into this, just not for 9.2.

@Naatan Naatan added this to the 9.3 milestone Jul 6, 2015

@ervumlens

This comment has been minimized.

ervumlens commented Jul 7, 2015

Good plan, and thanks to both of you for the quick update.

Without knowing the original motivation for the setTimeout call, it's hard to know the risks involved in pulling it. 😕

@Naatan Naatan modified the milestones: 9.3, Backlog Aug 6, 2015

@mitchell-as mitchell-as added the Minor label Sep 20, 2016

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Sep 20, 2016

Labelling this "Minor" because fixing this requires removing the 50ms timeout, which dates back to forever ago and it's really unclear what would happen if we removed it. Therefore this issue is very likely to remain unresolved.

@Naatan

This comment has been minimized.

Member

Naatan commented Sep 20, 2016

Given the current timeline it wouldn't be a bad idea to implement this now and essentially have months to see if there is any fallout. Reducing the timeout would also make typing feel more responsive I imagine?

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Sep 20, 2016

Not exactly. Typed characters and connected actions show up immediately. UI updates like styling take 50ms to show up.

@Naatan

This comment has been minimized.

Member

Naatan commented Sep 21, 2016

Gotcha. Still, given the timeline we're working with I wouldn't be opposed to trying this out. Nows as good a time as we'll ever have.

@Defman21

This comment has been minimized.

Contributor

Defman21 commented Sep 21, 2016

I'd want to test the version with decreased timeout. Some sort of a preference available from sdk/ko/prefs module would be very appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment