Skip to content

Conversation

@lonemadmax
Copy link

The default implementation to get a derived font with a different size just copies the platform data and sets the size. We need a new BFont, though, to avoid the size change in other text that is using the original.

FontPlatformData FontPlatformData::cloneWithSize(const FontPlatformData& source, float size)
{
FontPlatformData copy(source);
copy.m_font = std::make_shared<BFont>(source.font());
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't FontPlatformData copy constructor already do that? Then we could use the "default" method (although, as far as I can see, no one does, since it is disabled for both freetype and cocoa already).

Copy link
Author

Choose a reason for hiding this comment

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

shouldn't FontPlatformData copy constructor already do that? Then we could use the "default" method (although, as far as I can see, no one does, since it is disabled for both freetype and cocoa already).

That then implement their version as an exact copy of the default one, and they don't use it anyway, because they use the full constructor with the new size, instead of cloneWithSize, which, apart from Haiku, it is only used in Windows, and even then only for remote fonts.

Updated to do it in the copy constructor. Notice that, apart from the clone methods here, that's also used in the Font constructor. I opened a handful of pages and got about 100 to 1 calls to copy constructor versus updateSize, which in my inexperienced view feels a more natural place than this or the one from the previous patch, specially given

// updateSize to be implemented by each platform since it needs to re-instantiate the platform font object.
void updateSize(float);
.

I have the updateSize version in lonemadmax@85c6166, in case you change your mind. All three versions fix the issue.

Copy link
Member

Choose a reason for hiding this comment

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

The BFont copy constructor is trivial, so I think it shouldn't be a problem to invoke it more often?

The default implementation to get a derived font with a different size
just copies the platform data and sets the size. We need a new BFont,
though, to avoid the size change in other text that is using the
original.
@pulkomandy
Copy link
Member

pulkomandy commented Oct 22, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants