
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...)