Skip to content
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

Rework glyphs drawing. Use fixed font size measurements. #80

Merged
merged 16 commits into from Apr 18, 2020
Merged

Rework glyphs drawing. Use fixed font size measurements. #80

merged 16 commits into from Apr 18, 2020

Conversation

krzyzanowskim
Copy link
Contributor

@krzyzanowskim krzyzanowskim commented Apr 16, 2020

Fixes #67

  • revert to fixed height/width for size, yet still draw glyphs that cross the lines
  • disable selection view (again) and fixes performance issue when updating selection
  • update caret view position along the lines
  • draw underline
  • strikethrough doesn't (never did)

@krzyzanowskim krzyzanowskim marked this pull request as ready for review April 17, 2020 01:13
@krzyzanowskim krzyzanowskim marked this pull request as draft April 18, 2020 15:27
@krzyzanowskim krzyzanowskim changed the title Fix carrier position Rework glyphs drawing. Use fixed font size measurements. Apr 18, 2020
@@ -115,6 +114,9 @@ public class TerminalView: NSView, NSTextInputClient, NSUserInterfaceValidations

func setup(frame: CGRect, bounds: CGRect)
{
wantsLayer = true
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we use the layer for? Is this a good idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good question.

I used it to have a background without drawing because when drawing in context, it interferes with how selection/background color blending.

I didn't notice a side effect, but now I'm not sure

on one hand:

https://developer.apple.com/documentation/appkit/nsview/1483695-wantslayer

In a layer-backed view, any drawing done by the view is cached to the underlying layer object. This cached content can then be manipulated in ways that are more performant than redrawing the view contents explicitly.

on the other

https://developer.apple.com/documentation/appkit/nsview/1483686-drawrect

If your app manages content using its layer object instead, use the updateLayer method to update your layer instead of overriding this method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're good, after checking wantsUpdateLayer()

https://developer.apple.com/documentation/appkit/nsview/1483461-wantsupdatelayer

A view can update its contents using one of two techniques. It can draw those contents using its draw(:) method or it can modify its underlying layer object directly. During the view update cycle, each dirty view calls this method on itself to determine which technique to use. The default implementation of this method returns false, which causes the view to use its draw(:) method.

@migueldeicaza
Copy link
Owner

This looks very good, let me test it a little bit locally tonight.

@krzyzanowskim krzyzanowskim marked this pull request as ready for review April 18, 2020 18:25
@migueldeicaza
Copy link
Owner

This imrproves the rendering performance and fixes selection.

It makes the simple benchmark go from 0m0.827s to 0m0.538s.

@migueldeicaza migueldeicaza merged commit 069ad10 into migueldeicaza:master Apr 18, 2020
@migueldeicaza
Copy link
Owner

If we can turn off the layer, we could turn it off. Might be more resource friendly.

@migueldeicaza
Copy link
Owner

Well wantsLayer at least renders the bits below the screen, so I guess that is fine?

That said, I just added code to render under there, and that should have been triggering

@migueldeicaza
Copy link
Owner

With this new patch, this looks glorious:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Combining characters
2 participants