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

Fixed an issue where font metrics were incorrect for monospaced fonts. #8886

Merged
merged 12 commits into from
May 13, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,8 @@ else if (channelCount==3)
throw new FontFormatException("Could not generate font preview: " + e.getMessage());
}
}

boolean is_monospaced = true;
float base_advance = include_glyph_count > 0 ? glyphs.get(0).advance : 0;
for (int i = 0; i < include_glyph_count; i++) {
Glyph glyph = glyphs.get(i);
GlyphBank.Glyph.Builder glyphBuilder = GlyphBank.Glyph.newBuilder()
Expand All @@ -745,8 +746,13 @@ else if (channelCount==3)
}

glyphBankBuilder.addGlyphs(glyphBuilder);
if (base_advance != glyph.advance)
{
is_monospaced = false;
}
}

glyphBankBuilder.setIsMonospaced(is_monospaced);
glyphBankBuilder.setPadding(padding);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For non-monospaced fonts, this value taken into account as part of the last glyph width (we add padding*2 to each width). But for monospaced fonts, we need it to add it as part of the text width and also to offset it back when rendering monospaced font.

return previewImage;

}
Expand Down
30 changes: 19 additions & 11 deletions editor/src/clj/editor/font.clj
Original file line number Diff line number Diff line change
Expand Up @@ -162,17 +162,19 @@
:defold)
:distance-field))

(defn- measure-line [glyphs text-tracking ^String line]
(defn- measure-line [is-monospaced padding glyphs text-tracking ^String line]
(let [w (transduce (comp
(map #(:advance (glyphs (int %)) 0.0))
(interpose text-tracking))
+
0.0
line)
len (.length line)]
(if-let [last (get glyphs (and (pos? len) (int (.charAt line (dec len)))))]
(- (+ w (:left-bearing last) (:width last)) (:advance last))
w)))
(if is-monospaced
(+ w padding)
(if-let [last (get glyphs (and (pos? len) (int (.charAt line (dec len)))))]
(- (+ w (:left-bearing last) (:width last)) (:advance last))
w))))

(defn- split-text [glyphs ^String text line-break? max-width text-tracking]
(if line-break?
Expand Down Expand Up @@ -274,7 +276,7 @@
line-height (+ (:max-descent font-map) (:max-ascent font-map))
text-tracking (* line-height text-tracking)
lines (split-text glyphs text line-break? max-width text-tracking)
line-widths (map (partial measure-line glyphs text-tracking) lines)
line-widths (map (partial measure-line (:is-monospaced font-map) (:padding font-map) glyphs text-tracking) lines)
max-width (reduce max 0 line-widths)]
[max-width (* line-height (+ 1 (* text-leading (dec (count lines)))))]))))

Expand All @@ -301,7 +303,7 @@
line-height (+ (:max-descent font-map) (:max-ascent font-map))
text-tracking (* line-height text-tracking)
lines (split-text glyphs text line-break? max-width text-tracking)
line-widths (mapv (partial measure-line glyphs text-tracking) lines)
line-widths (mapv (partial measure-line (:is-monospaced font-map) (:padding font-map) glyphs text-tracking) lines)
max-width (reduce max 0 line-widths)]
(assoc text-layout
:width max-width
Expand Down Expand Up @@ -404,14 +406,20 @@
face-mask (if layer-mask-enabled
[1 0 0]
[1 1 1])
shadow-offset {:x (:shadow-x font-map), :y (:shadow-y font-map)}
shadow-offset {:x (if (:is-monospaced font-map)
(- (:shadow-x font-map) (* (:padding font-map) 0.5))
(:shadow-x font-map))
:y (:shadow-y font-map)}
AGulev marked this conversation as resolved.
Show resolved Hide resolved
alpha (:alpha font-map)
outline-alpha (:outline-alpha font-map)
shadow-alpha (:shadow-alpha font-map)]
;; Output glyphs per layer in back to front, if enabled but always output face layer.
shadow-alpha (:shadow-alpha font-map)
font-offset {:x (if (:is-monospaced font-map)
(- 0 (* (:padding font-map) 0.5))
0)
:y 0}]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We render each glyph starting some point and then draw every next letter at Advance Width (+ tracking) from prev. one.

But width of the glyph already include padding. Which makes width of the text a bit bigger and moves initial rendering point for the padding.

Adding padding the line-widths helps to return real line width and compensate 0.5 of padding for the positioning . But another 0.5 of padding should be compensted by moving the initial point of rendering (we can't just add double padding to the line-width)

AGulev marked this conversation as resolved.
Show resolved Hide resolved
(when (and layer-mask-enabled shadow-enabled) (fill-vertex-buffer-quads vbuf text-entries put-pos-uv-fn line-height char->glyph glyph-cache put-glyph-quad-fn [0 0 1] shadow-offset alpha outline-alpha shadow-alpha))
(when (and layer-mask-enabled outline-enabled) (fill-vertex-buffer-quads vbuf text-entries put-pos-uv-fn line-height char->glyph glyph-cache put-glyph-quad-fn [0 1 0] nil alpha outline-alpha shadow-alpha))
(fill-vertex-buffer-quads vbuf text-entries put-pos-uv-fn line-height char->glyph glyph-cache put-glyph-quad-fn face-mask nil alpha outline-alpha shadow-alpha)))
(when (and layer-mask-enabled outline-enabled) (fill-vertex-buffer-quads vbuf text-entries put-pos-uv-fn line-height char->glyph glyph-cache put-glyph-quad-fn [0 1 0] font-offset alpha outline-alpha shadow-alpha))
(fill-vertex-buffer-quads vbuf text-entries put-pos-uv-fn line-height char->glyph glyph-cache put-glyph-quad-fn face-mask font-offset alpha outline-alpha shadow-alpha)))

(defn gen-vertex-buffer
[^GL2 gl {:keys [type font-map] :as font-data} text-entries]
Expand Down
27 changes: 21 additions & 6 deletions editor/src/clj/editor/pipeline/fontc.clj
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@
(defn- make-glyph-data-bank ^bytes [glyph-extents]
(byte-array (transduce (map :glyph-data-size) + 0 glyph-extents)))

(defn check-monospaced [semi-glyphs]
(let [base-advance (if-let [first-glyph (first semi-glyphs)]
(:advance first-glyph)
0)]
(every? #(= base-advance (:advance %)) semi-glyphs)))

(def ^:private positive-glyph-extent-pairs-xf (comp (map pair) (filter (comp positive-wh? :glyph-wh second))))

(defn- compile-fnt-bitmap [font-desc font-resource resolver]
Expand All @@ -173,7 +179,8 @@
line-height (+ cache-cell-max-ascent cache-cell-max-descent)
cache-cell-wh (max-glyph-cell-wh glyph-extents line-height padding glyph-cell-padding)
cache-wh (cache-wh font-desc cache-cell-wh (count semi-glyphs))
glyph-data-bank (make-glyph-data-bank glyph-extents)]
glyph-data-bank (make-glyph-data-bank glyph-extents)
is-monospaced (check-monospaced semi-glyphs)]

(doall
(pmap (fn [[semi-glyph glyph-extents]]
Expand Down Expand Up @@ -225,7 +232,9 @@
:cache-cell-height (:height cache-cell-wh)
:cache-cell-max-ascent max-ascent
:glyph-channels channel-count
:glyph-data (ByteString/copyFrom glyph-data-bank)})))
:glyph-data (ByteString/copyFrom glyph-data-bank)
:is-monospaced is-monospaced
:padding padding})))

(defn- do-blend-rasters [^Raster src ^Raster dst-in ^WritableRaster dst-out]
(let [width (min (.getWidth src) (.getWidth dst-in) (.getWidth dst-out))
Expand Down Expand Up @@ -438,7 +447,8 @@
(update :width #(* (int (Math/ceil (/ ^double % 4.0))) 4)))
cache-wh (cache-wh font-desc cache-cell-wh (count semi-glyphs))
glyph-data-bank (make-glyph-data-bank glyph-extents)
layer-mask (font-desc->layer-mask font-desc)]
layer-mask (font-desc->layer-mask font-desc)
is-monospaced (check-monospaced semi-glyphs)]
(doall
(pmap (fn [[semi-glyph glyph-extents]]
(let [^BufferedImage glyph-image (let [face-color (Color. ^double (:alpha font-desc) 0.0 0.0)
Expand Down Expand Up @@ -494,7 +504,9 @@
:glyph-data (ByteString/copyFrom glyph-data-bank)
:alpha (:alpha font-desc)
:outline-alpha (:outline-alpha font-desc)
:shadow-alpha (:shadow-alpha font-desc)}))
:shadow-alpha (:shadow-alpha font-desc)
:is-monospaced is-monospaced
:padding padding}))

(defn- calculate-ttf-distance-field-edge-limit [^double width ^double spread ^double edge]
(let [sdf-limit-value (- (/ width spread))]
Expand Down Expand Up @@ -640,7 +652,8 @@
cache-cell-wh (max-glyph-cell-wh glyph-extents line-height padding glyph-cell-padding)
cache-wh (cache-wh font-desc cache-cell-wh (count semi-glyphs))
glyph-data-bank (make-glyph-data-bank glyph-extents)
layer-mask (font-desc->layer-mask font-desc)]
layer-mask (font-desc->layer-mask font-desc)
is-monospaced (check-monospaced semi-glyphs)]
(doall
(pmap (fn [[semi-glyph glyph-extents]]
(let [{:keys [^bytes field]} (draw-ttf-distance-field semi-glyph padding channel-count outline-width sdf-outline sdf-spread sdf-shadow-spread edge shadow-blur shadow-alpha)
Expand Down Expand Up @@ -685,7 +698,9 @@
:glyph-data (ByteString/copyFrom glyph-data-bank)
:alpha (:alpha font-desc)
:outline-alpha (:outline-alpha font-desc)
:shadow-alpha (:shadow-alpha font-desc)}))
:shadow-alpha (:shadow-alpha font-desc)
:is-monospaced is-monospaced
:padding padding}))

(defn compile-font [font-desc font-resource resolver]
(let [font-ext (resource/type-ext font-resource)]
Expand Down
14 changes: 8 additions & 6 deletions engine/engine/src/test/def-3575/def-3575.script
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ function init(self)
end

function update(self, dt)
-- Values are taken from the output from label.get_text_metrics from defold version 1.2.138
-- Values are taken from the output from resource.get_text_metrics from defold version 1.2.138
-- The base settings used are:
-- font: "/builtins/fonts/vera_mo_bd.ttf"
-- font: "/builtins/fonts/vera_mo_bd.ttf" - monospaced
-- size: 14
-- antialias: 1
-- alpha: 1.0
Expand All @@ -35,9 +35,11 @@ function update(self, dt)
-- shadow_alpha: 1.0
-- shadow_blur: 2

local function test_label_metrics(label_name,test_metrics)
local function test_label_metrics(label_name, test_metrics)
-- Test that we are getting the expected value back from get_text_metrics
local metrics = label.get_text_metrics("#" .. label_name)
local comp = "#" .. label_name
local font = go.get(comp, "font")
local metrics = resource.get_text_metrics(font, label.get_text(comp))

local result = test_metrics.max_ascent == metrics.max_ascent and
test_metrics.max_descent == metrics.max_descent and
Expand All @@ -54,7 +56,7 @@ function update(self, dt)
local function test_label_bitmap()
local metrics_valid = {
max_ascent = 13,
width = 781,
width = 774,
height = 17,
max_descent = 4
}
Expand All @@ -65,7 +67,7 @@ function update(self, dt)
local function test_label_df()
local metrics_valid ={
max_ascent = 13,
width = 783,
width = 775,
height = 17,
max_descent = 4,
}
Expand Down
2 changes: 2 additions & 0 deletions engine/gamesys/src/gamesys/resources/res_font_map.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ namespace dmGameSystem
params.m_ImageFormat = glyph_bank->m_ImageFormat;
params.m_GlyphChannels = glyph_bank->m_GlyphChannels;
params.m_GlyphData = glyph_bank->m_GlyphData.m_Data;
params.m_IsMonospaced = glyph_bank->m_IsMonospaced;
params.m_Padding = glyph_bank->m_Padding;

if (font_map == 0)
{
Expand Down
3 changes: 3 additions & 0 deletions engine/render/proto/render/font_ddf.proto
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ message GlyphBank
optional uint32 cache_cell_width = 14;
optional uint32 cache_cell_height = 15;
optional uint32 cache_cell_max_ascent = 16;

optional uint32 padding = 17 [default = 0];
optional bool is_monospaced = 18 [default = false];
}

message FontMap
Expand Down
39 changes: 29 additions & 10 deletions engine/render/src/render/font_renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ namespace dmRender
, m_CacheCellPadding(0)
, m_LayerMask(FACE)
, m_ImageFormat(dmRenderDDF::TYPE_BITMAP)
, m_IsMonospaced(false)
, m_Padding(0)
{

}
Expand Down Expand Up @@ -111,6 +113,8 @@ namespace dmRender
, m_CacheCellMaxAscent(0)
, m_CacheCellPadding(0)
, m_LayerMask(FACE)
, m_IsMonospaced(false)
, m_Padding(0)
{

}
Expand Down Expand Up @@ -162,6 +166,8 @@ namespace dmRender
uint32_t m_CacheCellMaxAscent;
uint8_t m_CacheCellPadding;
uint8_t m_LayerMask;
uint8_t m_IsMonospaced:1;
uint8_t m_Padding:7;
};

static float GetLineTextMetrics(HFontMap font_map, float tracking, const char* text, int n, bool measure_trailing_space);
Expand Down Expand Up @@ -235,6 +241,8 @@ namespace dmRender
uint32_t cell_count = font_map->m_CacheColumns * font_map->m_CacheRows;

font_map->m_CellTempData = (uint8_t*)malloc(font_map->m_CacheCellWidth*font_map->m_CacheCellHeight*4);
font_map->m_IsMonospaced = params.m_IsMonospaced;
font_map->m_Padding = params.m_Padding;

switch (params.m_GlyphChannels)
{
Expand Down Expand Up @@ -325,6 +333,8 @@ namespace dmRender
font_map->m_OutlineAlpha = params.m_OutlineAlpha;
font_map->m_ShadowAlpha = params.m_ShadowAlpha;
font_map->m_LayerMask = params.m_LayerMask;
font_map->m_IsMonospaced = params.m_IsMonospaced;
font_map->m_Padding = params.m_Padding;

font_map->m_CacheWidth = params.m_CacheWidth;
font_map->m_CacheHeight = params.m_CacheHeight;
Expand Down Expand Up @@ -515,7 +525,7 @@ namespace dmRender
center_x += metrics.m_Width/2; // move halfway to the right since we're aligning left
break;
case TEXT_ALIGN_RIGHT:
center_x -= metrics.m_Height/2; // move halfway to the left from pivot since we're aligning right
center_x -= metrics.m_Width/2; // move halfway to the left from pivot since we're aligning right
break;
// nothing to do for TEXT_ALIGN_CENTER. Pivot is already at the center of the text X-wise
}
Expand Down Expand Up @@ -739,6 +749,10 @@ namespace dmRender
float layout_width;
int line_count = Layout(text, width, lines, max_lines, &layout_width, lm, measure_trailing_space);
float x_offset = OffsetX(te.m_Align, te.m_Width);
if (font_map->m_IsMonospaced)
{
x_offset -= font_map->m_Padding * 0.5f;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

please check the comment for clojure code.

}
float y_offset = OffsetY(te.m_VAlign, te.m_Height, font_map->m_MaxAscent, font_map->m_MaxDescent, te.m_Leading, line_count);

const Vector4 face_color = dmGraphics::UnpackRGBA(te.m_FaceColor);
Expand Down Expand Up @@ -835,8 +849,8 @@ namespace dmRender

for (int line = 0; line < line_count; ++line) {
TextLine& l = lines[line];
int16_t x = (int16_t)(x_offset - OffsetX(te.m_Align, l.m_Width) + 0.5f);
int16_t y = (int16_t) (y_offset - line * leading + 0.5f);
Copy link
Contributor Author

@AGulev AGulev May 7, 2024

Choose a reason for hiding this comment

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

Rounded vertex coordinates is the reason of dancing text (#7197)

float x = x_offset - OffsetX(te.m_Align, l.m_Width);
float y = y_offset - line * leading;
const char* cursor = &text[l.m_Index];
int n = l.m_Count;
for (int j = 0; j < n; ++j)
Expand Down Expand Up @@ -1015,7 +1029,7 @@ namespace dmRender
vertexindex += vertices_per_quad;
}
}
x += (int16_t)(g->m_Advance + tracking);
x += g->m_Advance + tracking;
}
}

Expand Down Expand Up @@ -1201,15 +1215,20 @@ namespace dmRender
}

last = g;
// NOTE: We round advance here just as above in DrawText
width += (int16_t) (g->m_Advance + tracking);
width += g->m_Advance + tracking;
}
if (n > 0 && 0 != last)
{
uint32_t last_width = (measure_trailing_space && last->m_Character == ' ') ? (int16_t)last->m_Advance : last->m_Width;
float last_end_point = last->m_LeftBearing + last_width;
float last_right_bearing = last->m_Advance - last_end_point;
width = width - last_right_bearing - tracking;
if (font_map->m_IsMonospaced) {
width += font_map->m_Padding;
}
else {
uint32_t last_width = (measure_trailing_space && last->m_Character == ' ') ? last->m_Advance : last->m_Width;
float last_end_point = last->m_LeftBearing + last_width;
float last_right_bearing = last->m_Advance - last_end_point;
width = width - last_right_bearing;
}
width -= tracking;
}

return width;
Expand Down
2 changes: 2 additions & 0 deletions engine/render/src/render/font_renderer.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ namespace dmRender
uint32_t m_CacheCellMaxAscent;
uint8_t m_CacheCellPadding;
uint8_t m_LayerMask;
uint8_t m_IsMonospaced:1;
uint8_t m_Padding:7;

dmRenderDDF::FontTextureFormat m_ImageFormat;
};
Expand Down