fix: replace int() with round() to implement rounding in scale_bounding_box()

[GoldPigg]

Description

The following function is used to simulate transform, so its comment mentions "round values for better simulation". However, its implementation uses int() instead of round().

nerd-fonts/font-patcher

Lines 1943 to 1955 in a861c72

	def scale_bounding_box(bbox, scale_x, scale_y):
	""" Return a scaled version of a glyph dimensions dict """
	# Simulate scaling on combined bounding box, round values for better simulation
	new_dim = {
	'xmin' : int(bbox['xmin'] * scale_x),
	'ymin' : int(bbox['ymin'] * scale_y),
	'xmax' : int(bbox['xmax'] * scale_x),
	'ymax' : int(bbox['ymax'] * scale_y),
	'advance': int(bbox['advance'] * scale_x) if bbox['advance'] is not None else None,
	}
	new_dim['width'] = new_dim['xmax'] + (-new_dim['xmin'])
	new_dim['height'] = new_dim['ymax'] + (-new_dim['ymin'])
	return new_dim

This may introduce minor errors (results being 1 less than transform). While this likely has negligible impact on final outcomes, it can cause confusion during debugging—leaving developers(eg. me) scratching their heads over two values that should be equal but consistently differ by 1.

Requirements / Checklist

• Read the Contributing Guidelines
• I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan.
  Issue number where discussion took place: #xxx
• If this contains a font/glyph add its origin as background info below (e.g. URL)
• Verified the license of any newly added font, glyph, or glyph set. License is: xxx

What does this Pull Request (PR) do?

Replace int() with round() to implement rounding in scale_bounding_box()

How should this be manually tested?

When patching Go-Mono.ttf, glyph 0x259f has the same bounding box as scale_data. In this specific case, the expected outcome is that scale_bounding_box() should yield the same result as sys_dim. However, sym_dim does not equal scaleglyph_dim. This can be tested as shown in the image below.

Any background context you can provide?

What are the relevant tickets (if any)?

Screenshots (if appropriate or helpful)

[Finii]

Thanks for this interesting PR! 💚

After having spent some time on this, I wonder if we should round there at all.

What I have found:

• Scaling the glyph by some factor and then taking the .boundingBox() always returns integer bounds, rounded normally
• Shifting the glyph by some non-integer distance results in a .boundingBox() with non-integer bounds
• I did just try the same font you did, a ttf variant that allows only integer point coordinates
• We DO round the point coordinates for otf fonts after all transforms explicitly (but not in between)
• The whole purpose of the code as referenced by you is to determine the SHIFT of the already copy&pasted scaled glyph with respect to some virtual BB that might be bigger. Rounding in the frame of the virtual BB does not really make sense as only the real glyph points will be rounded which are on (generally) different values
• so for example for centered glyphs when we are off by 1 EM units, that means a shift difference of 1/1000 to 1/2000 of an EM unit in shift. Which will be rounded away probably ;)

Looking at the more complicated scale code that is farther up, I believe that also there it is better to use the unrounded values. The point coordinates are checked after real scaling anyhow 🤔

So we have this

# Drop nonintegral part of nodes' coordinates; ttf will do it anyhow, otf will be much smaller
self.sourceFont[currentSourceFontGlyph].round()

and

if scale_ratio_x != 1.0 or scale_ratio_y != 1.0:
    # scale a tiny bit too small to avoid rounding problems
    scale_ratio_x *= self.sourceFont.em / (self.sourceFont.em + 1)

which should take care of most rounding issues.

The rounding down (i.e. int()) has been introduced 2022-10-13.
While the other mentioned mechanisms were added 2025-04-23.

What do you think, is not-rounding at all not the better solution?

---

I remember writing that int() code a bit, and one rationale was that .boundingBox() always returns integers and that should probably do the same. But even that seems to be a wrong assumption. With 'virtual' bounding boxes it finally does not make any sense anymore as the virtual coordinates might or might not fall onto real coordinates.

[Finii]

With 'virtual' bounding boxes [...] coordinates might or might not fall onto real coordinates.

That is not true, the extrema of the virtual bounding box is always some real node. 🤔

[Finii]

Approve, but maybe you have time to think about dropping any rounding.

[GoldPigg]

Thank you for your review!❤
I've looked at the relevant code again and have some new thoughts on the rounding issue:

• To minimize code changes, scale_bounding_box() should perform rounding. Because subsequent shifting operations rely on the result after .round(), and this function should still simulate the outcome after actual scaling (i.e., after rounding).

  Consider this specific case: a glyph forming its own group. Under current logic, its subsequent shifting should naturally be based on the rounded result. If we omit rounding in scale_bounding_box(), its output could defferent from the actual behavior (i.e., the result after .round()).

• If we were truly “obsessed” with pursuing accuracy (like a “nerd”), perhaps we should move rounding operations like .round() to after shifting, and avoid rounding during earlier steps including scale_bounding_box().
  It's crucial to note, as you mentioned:

  Scaling the glyph by some factor and then taking the .boundingBox() always returns integer bounds, rounded normally

  I reviewed the relevant FontForge documentation, and it appears we need to pass a round parameter to disable FontForge's automatic rounding.

In summary, the first approach is convenient but sacrifice some precision. The second method offers accuracy but require significant changes and potentially encounter unforeseen challenges.

So, what do you think?🤔

[Finii]

Thanks for your thoughts!

When reading the code again (getting rid of all rounding related problems was/is really hard), I stumbled about this code that I had completely forgotten:

if self.args.single:
    # Check and correct the scaling after rounding (if all 3 tries fail we will get a warning later on)
    destmaxsize = self.font_dim['width'] * max(1, 1 + (overlap or 0))
    for increaser in range(3):

The problem is that fontforge will round internally to some unknown precision always. The internal node coordinates (floating point) have some precision, that is not double, maybe it is float, maybe even less. If we call transform the result will be already rounded to the (maybe) 4th digit, i.e. 1234.567891234 becomes 1234.5679. And that number is than rounded by .round(). The comment tells us I encountered problems with even 3 iterations were not enough.

Underlying is that the node coordinates are rounded, but the bounding box is not always determined by nodes. The nodes describe the quadratic or cubic Bezier curves and THOSE can extend well beyond their defining nodes. That is forbidden discouraged for fonts and they should always have a node on the extreme outside (bounding box) - but all the icons we add, they come from SVGs and there no such limit exists and we can not correct that automatically.
So if we round the nodes the bounding box still might extend outward from all node coordinates and by some fractional amount.
That is the reason why we try different scales in the self.args.single variant. For 'normal' fonts we do not care so much and just live with a bounding box that will be off by one or two units 😬

The glyph 259Fis completely rectangular and does not have any Beziers on the outside, so there it is no problem.

Nodes at bounding box extrema

Rounding problems with the extrema

Detail on the left hand side 😞

---

I will run some final check later and then pull your excellent change 👍
Thank you again for the valuable input!

[Finii]

seti-checkbox

The left hand side point (open circle) is not a node. The nodes are filled circles.
The actual bounding box relies on more than one rounding operation here, the node coordinates and node handle coordinates, individually rounded, means 4 rounded entities make up the bounding box extremum.

[GoldPigg]

Thank you for your active interest and for sharing some fascinating details with me.
And if my ideas can be of help to you, I'd be truly delighted φ(゜▽゜*)♪.

[Finii]

@allcontributors pleas add @GoldPigg for code

[allcontributors]

@Finii

I've put up a pull request to add @GoldPigg! 🎉