Skip to content

Added FT_StreamRec_ fields to WIN64 patch - #5

Open
MnemonicWME wants to merge 1 commit into
Robmaister:masterfrom
MnemonicWME:master
Open

Added FT_StreamRec_ fields to WIN64 patch#5
MnemonicWME wants to merge 1 commit into
Robmaister:masterfrom
MnemonicWME:master

Conversation

@MnemonicWME

Copy link
Copy Markdown

size and pos fields of FT_StreamRec_ also need special treatment on Win64.

@HinTak

HinTak commented May 17, 2017

Copy link
Copy Markdown

The change is incomplete. The __int64 type needs the config header, so the below is needed also. I assume you must have built a minimal/smaller freetype or or not have this module configured to miss this?

--- old_src/cid/type1cid.c
+++ src/cid/type1cid.c
@@ -18,6 +18,7 @@
 
 #define FT_MAKE_OPTION_SINGLE_OBJECT
 #include <ft2build.h>
+#include FT_CONFIG_CONFIG_H
 
 #include "cidgload.c"
 #include "cidload.c"

@MnemonicWME

Copy link
Copy Markdown
Author

It's not in the original file either. I merely added new fields that also need to be __int64 instead of long on Win64.

@HinTak

HinTak commented May 17, 2017

Copy link
Copy Markdown

It is newly needed, because your addition is used by one of the cid* source files. I assume you did not build the cid module.

@Robmaister

Copy link
Copy Markdown
Owner

@HinTak I believe @MnemonicWME is making the (correct) assumption that this patch only needs to compile in MSVC, which provides both the _WIN64 macro and __int64 types without the addition of any headers. Windows is the only major OS to make long 32-bits wide instead of 64-bits wide, and this is the only thing that the patch is meant to address.

If we come across someone who happens to be using another OS with a LLP64 data model, then we can broaden the scope of the patch.

That is, unless I'm missing something? The patch is very simple and appears to have no effect unless we're compiling for Windows x64, and in that case we're only using types made available to us by the compiler.

Other than that, this was a very good catch - I'll merge it in, merge your other PR, and build new versions of freetype6.dll when I get the chance to do so.

@HinTak

HinTak commented May 17, 2017

Copy link
Copy Markdown

Yes, I routinely cross-compile freetype for windows with the mingw gcc compiler. See my post to freetype-devel on this patch, specifically:

http://lists.nongnu.org/archive/html/freetype-devel/2017-05/msg00073.html

The first part of the e-mail eventually became the last code change in freetype 2.8 before it was released. The 2nd part (i.e. this) did not make it.

@HinTak

HinTak commented May 17, 2017

Copy link
Copy Markdown

Btw, SharpFont HEAD is really broken as far as building with mono is concerned, so I have already backported the C# part of this change to the 4.0.1 fork on mine. Backporting is easier than looking at the brokenness. I hope you fix the brokenness before the next release.

@MnemonicWME

Copy link
Copy Markdown
Author

Please correct me if I'm wrong, but I think we're having bit of a clash of two approaches. While @HinTak is trying to push the necessary patches to freetype, the original approach of @Robmaister was just to patch the existing source to suit the needs of SharpFont. Because, all these changes are only really necessary for interop with .NET on Win64, which I believe is not a primary concern of freetype :)
I adopted the latter approach, because it will work for older freetype releases, while the former approach is probably better for the long term.

As for my changes, I admit I only tried to compile freetype 2.5.5. From what I've seen, the current versions use a different file structure, so the patch will probably fail for them anyway (?).

@Robmaister

Copy link
Copy Markdown
Owner

Ah, it appears mingw-w64 (and probably also other variants) define _WIN64 for us, but don't have __int64. If it doesn't break anything on the MSVC side/have any side effects, I don't see why it shouldn't be included. MinGW follows data sizes for Windows there as well (long is 4 bytes, not 8).

Ideally these patches (or more generalized ones) would end up in freetype. The scope that I care about is MSVC only (as those are the builds I'm providing) but, if caught in time, mingw issues should be fixed as well. Ideally we'll end up in a spot where these patches don't have to exist at all and we can just use any normal build of freetype >= vX.Y.Z

If the file structure has changed, then we'll need new patches - I'll get to this when I have a chance.

@HinTak

HinTak commented May 18, 2017

Copy link
Copy Markdown

I have already had a version of the patch that works with freetype 2.8 (as suggested in my freetype-devel post). Just needing time to test the C# side of things.

The upstream code around the patch is a bit wrong as it doesn't allow font files over 2GB on windows. But I agree with Werner that the change does not belong in freetype, as all it does is to cater for a limitation in c#. None the less, the upstream code around the stream interface needs to change at some point in the future as it does not allow for fonts over 2GB for windows, either 32-bit or 64-bit.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants