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.

Sign up for GitHub

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

Jump to bottom

Emoji rendering in DWrite shaper & DWrite font locator #360

Merged
merged 5 commits into from
Aug 1, 2021
Merged

Emoji rendering in DWrite shaper & DWrite font locator #360

merged 5 commits into from
Aug 1, 2021

Conversation

skyline75489
Copy link
Contributor

No description provided.

@github-actions github-actions bot added fonts font rasterization and text shaping API and platform implementations frontend Contour Terminal Emulator (GUI frontend) VT: rasterizer Rendering of the terminal into a pixmap using `terminal_renderer` library labels Jul 31, 2021
skyline75489
skyline75489 commented Jul 31, 2021
View reviewed changes
src/text_shaper/directwrite_shaper.cpp Outdated Show resolved Hide resolved
src/text_shaper/directwrite_shaper.cpp Show resolved Hide resolved
src/terminal_renderer/TextRenderer.cpp Outdated Show resolved Hide resolved
src/text_shaper/directwrite_shaper.cpp Show resolved Hide resolved
christianparpart
christianparpart reviewed Jul 31, 2021
View reviewed changes
src/text_shaper/directwrite_shaper.cpp
tmp.size()
);

for (auto i = 0; i < height; i++)
Copy link
Member

Choose a reason for hiding this comment

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

Would you want to give range::views::iota(0, N) a try? An example use would be here:

contour/src/terminal/Grid.cpp

Lines 557 to 558 in abdae01

for (auto const i: ranges::views::iota(0u, n.as<size_t>()))
resetLine(baseLineOffset + LineOffset::cast_from(i), _defaultAttributes);

It avoids the need of having the iterator variable being noon-const (mutable) even though it is only used read-only in the loop body.

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 am too dumb to actually make it work 😅 .

The compiler hates me. I'll give it a try in future PRs.

@skyline75489 skyline75489 changed the title Draft fix for emoji rendering in DWrite shaper Emoji rendering in DWrite shaper & DWrite font locator Aug 1, 2021
skyline75489
skyline75489 commented Aug 1, 2021
View reviewed changes
src/text_shaper/directwrite_locator.cpp Show resolved Hide resolved
src/text_shaper/directwrite_locator.cpp Show resolved Hide resolved
src/text_shaper/directwrite_shaper.cpp
ComPtr<IDWriteTextFormat1> format1;

if (SUCCEEDED(hr) && SUCCEEDED(format->QueryInterface(IID_PPV_ARGS(&format1))))
font_source_list sources = d->locator_->resolve(gsl::span(_text.data(), _text.size()));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that only DWrite locator implements resolve. So basically you'll see zero font fallback for CJK if using DWrite shaper + Font config locator.

Also I think resolve here need to also pass fontDescription for bold/italic/etc.

Copy link
Member

Choose a reason for hiding this comment

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

I will catch up with them font config "resolve" implementation ASAP :)

src/text_shaper/open_shaper.cpp Show resolved Hide resolved
@christianparpart christianparpart mentioned this pull request Aug 1, 2021
Open
christianparpart
christianparpart requested changes Aug 1, 2021
View reviewed changes
Copy link
Member

@christianparpart christianparpart left a comment

Choose a reason for hiding this comment

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

I was a little more strict in reviewing. Most of it I think you can just take as suggestions and/or thoughts. :)

src/terminal_renderer/Renderer.cpp Outdated Show resolved Hide resolved
src/terminal_renderer/Renderer.cpp Outdated Show resolved Hide resolved
src/text_shaper/directwrite_analysis_wrapper.h Outdated Show resolved Hide resolved
src/text_shaper/directwrite_analysis_wrapper.h Outdated Show resolved Hide resolved
src/text_shaper/directwrite_locator.cpp Show resolved Hide resolved
src/text_shaper/directwrite_shaper.cpp Outdated Show resolved Hide resolved
src/text_shaper/directwrite_shaper.h Outdated Show resolved Hide resolved
src/text_shaper/font.h Outdated Show resolved Hide resolved
src/text_shaper/fontconfig_locator.h Show resolved Hide resolved
src/text_shaper/open_shaper.cpp Show resolved Hide resolved
@christianparpart christianparpart marked this pull request as ready for review August 1, 2021 15:52
@christianparpart christianparpart merged commit 31f06c9 into contour-terminal:master Aug 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christianparpart christianparpart christianparpart requested changes

Assignees
No one assigned
Labels
fonts font rasterization and text shaping API and platform implementations frontend Contour Terminal Emulator (GUI frontend) VT: rasterizer Rendering of the terminal into a pixmap using `terminal_renderer` library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@skyline75489 @christianparpart