fix: SVG 폰트 폴백 체인 — 영문 세리프 폰트명 인식 개선 (#616)#820
Open
oksure wants to merge 2 commits into
Open
Conversation
- "serif"를 포함하지만 "sans"를 포함하지 않는 폰트명(Liberation Serif, Noto Serif CJK 등)을 세리프 폴백 체인으로 올바르게 라우팅 - "mono"를 포함하는 폰트명을 고정폭 폴백 체인으로 라우팅 - char_overlap 렌더링에서도 generic_fallback() 사용하도록 통일 (기존: 단순 ",sans-serif" 하드코딩) 이전에는 "Noto Serif CJK SC" 같은 세리프 폰트가 sans-serif 폴백을 받아서, 폰트 미설치 환경에서 시각적 불일치가 발생했음. Ref edwardkim#616
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes incorrect SVG font fallback selection when the primary font name includes English keywords like “Serif” (and improves handling for “Mono”), ensuring more appropriate serif/monospace fallback chains in font-missing environments. This aligns SVG rendering behavior with the existing generic_fallback() logic rather than hardcoding ,sans-serif for char-overlap rendering.
Changes:
- Update
generic_fallback()to treat names containingserif(but notsans) as serif, and names containingmonoas monospace. - Update SVG CharOverlap rendering to append the computed
generic_fallback()chain instead of always appendingsans-serif.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/renderer/svg.rs |
Switch CharOverlap font-family construction from hardcoded ,sans-serif to generic_fallback()-based chaining. |
src/renderer/mod.rs |
Improve keyword detection in generic_fallback() for English serif fonts and monospace (“mono”) fonts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
556
to
560
| if font_family.contains("굴림체") || font_family.contains("바탕체") | ||
| || lower.contains("gulimche") || lower.contains("batangche") | ||
| || lower.contains("coding") || lower.contains("courier") | ||
| || lower.contains("mono") | ||
| { |
Comment on lines
573
to
578
| // 세리프 키워드 (영문) — "serif" 포함 but "sans-serif" 제외 | ||
| if lower.contains("times") || lower.contains("hymjre") | ||
| || lower.contains("palatino") || lower.contains("georgia") | ||
| || lower.contains("batang") || lower.contains("gungsuh") | ||
| || (lower.contains("serif") && !lower.contains("sans")) | ||
| { |
Copilot 리뷰 반영:
- Noto Serif CJK SC, Liberation Serif, Noto Sans Mono 등 신규 케이스 테스트 추가
- "sans" 포함 폰트가 세리프로 오분류되지 않는지 검증
- 주석을 코드 동작과 일치하도록 정정 ("sans-serif" → "sans" 부분문자열)
Contributor
Author
|
Copilot 리뷰 반영 (ec96be9):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
요약
generic_fallback()이 영문 세리프/고정폭 폰트명을 제대로 인식하지 못해, 폰트 미설치 환경에서 잘못된 폴백 체인이 적용되던 문제를 수정합니다.문제
Noto Serif CJK SC,Liberation Serif등 영문명에 "Serif"를 포함하는 폰트가 sans-serif 폴백을 받음generic_fallback()대신 하드코딩된,sans-serif사용수정
renderer/mod.rsrenderer/mod.rsgeneric_fallback()사용renderer/svg.rs검증
freedomofpress/dangerzone-image 샘플로 테스트:
Ref #616
감사합니다.