Re: [Inkscape-devel] Freetype2/win32 conversion
Jon A. Cruz wrote:
Bob Jamison wrote:
It works! Please try downloading this version:
(it was an error in FontFactory initializing the wrong font engine on Win32. Not FontInstance's fault)
Hmmm... was it the kind of thing that might have been caught sooner if variables/structs in question were set to initial values such as null?
I would say that it was a subtle error, though after you notice it, it is obvious, like "why didn't I see that?"
In FontFactory.cpp, there were #ifdef switches, like this:
#ifdef WITH_XFT ...code... #endif
#ifdef WIN32 ....code... #endif
Its structure should have been like in FontInstance.cpp, #ifdef WITH_XFT ...code... #elif defined(WIN32) ...code... #endif
...since the definitions are not mutually exclusive.
Thus the font server was being set to win32 in FontFactory, and FT2 was being called in FontInstance.
Obvious -after- you see it.
I'd say that this was 2/3 of the problem, and the wrong fontconfig DLL being called by Inkscape was the other 1/3.
Bob
Bob Jamison wrote:
I would say that it was a subtle error, though after you notice it, it is obvious, like "why didn't I see that?"
In FontFactory.cpp, there were #ifdef switches, like this:
#ifdef WITH_XFT ...code... #endif
#ifdef WIN32 ....code... #endif
Its structure should have been like in FontInstance.cpp, #ifdef WITH_XFT ...code... #elif defined(WIN32) ...code... #endif
...since the definitions are not mutually exclusive.
Eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeek!!!
(Man, I really have to get my new HD installed and get the latest sources)
That's where I think a better ifdef scheme is needed. We probably should try to get to that ASAP as it's not just hypothetically better, but would help with subtle bugs we're encountering already.
Basically, the problem is compounded by mixing platform checks with feature checks. I'm a strong proponent of all platform checks belonging only in config.h. Anything else should use feature checks in the code instead.
#ifdef USE_XFT ...code... #elif defined(USE_WIN32_FNT) ...code... #else #warning "Unsupported platform" #endif
Notice the switch from the platform define to a USE_ one, and also the default case being added (though I think that "#warning" is gcc only...)
Jon A. Cruz wrote:
That's where I think a better ifdef scheme is needed. We probably should try to get to that ASAP as it's not just hypothetically better, but would help with subtle bugs we're encountering already.
Basically, the problem is compounded by mixing platform checks with feature checks. I'm a strong proponent of all platform checks belonging only in config.h. Anything else should use feature checks in the code instead.
#ifdef USE_XFT ...code... #elif defined(USE_WIN32_FNT) ...code... #else #warning "Unsupported platform" #endif
Notice the switch from the platform define to a USE_ one, and also the default case being added (though I think that "#warning" is gcc only...)
I agree. It is so easy to forget why there is an #ifdef WIN32 in the code in the first place. This last week's episode of "Pangoification, Part II" is a good example.
This is somewhat moot for libnrtype, since the pango_win32* sections of the Pango code are disappearing. (They were out of sync with the FT2 code anyway).
Bob
participants (2)
-
Bob Jamison -
Jon A. Cruz