On Tue, May 07, 2024 at 05:58:55PM +0000, Thorsten Glaser wrote: > Dmitry Shachnev dixit: > > >Will you be able to forward your patch upstream when you finalize it? > > Sort of. I can do the CLA, Gerrit, etc. dance, but I probably cannot > respond well if they want me to test things with vanilla upstream > (instead of the packaging), or if they have requests I as a C (but > not C++) developer don’t understand.
I also usually test with our packaging, not with vanilla upstream. But with Qt 6.6 from experimental there should not be much difference: we don't have any patches for this part of code, and we are lagging only a few versions behind upstream. And your test example compiles without changes with Qt 6, you just need to call qmake6 instead of qmake. > >> +static inline int roundForBbox(qreal v) > >> +{ > >> + return (v < 0) ? floor(v) : ceil(v); > >> +} > > > >I see you are passing negated values to this function, e.g. roundForBbox(-x). > > Not only them. And roundForBbox is basically just the canonical > “round away from zero / towards ±infinity for integer results”. > > The negation in the callers is to convert *some* Qt coordinates > to PS/PDF coordinates, but only for the Y axis, as the X axis is > the same for them. Okay, makes sense. > >I see why you moved properties to the top, but is moving postscriptName > >needed? Maybe leave it where it was to minimize the diff? > > It’s not. I moved the whole block to make it easier to read, > but good point. Thank you. > >You are passing scalep pointer here only to multiply it by this value? > > Yes. > > >It looks like fontEngine is a public member of QFontSubset, so you can > >do it in the calling code. > > Yes, except it’s the callee that determines the scaling factor, > not the caller. By passing it back like this, nothing would have > to change in the caller should the callee ever decide to not scale. Valid point. Let's see if Qt developers like this approach with passing a pointer. If they don't, maybe the same thing can be achieved differently, e.g. by returning QPair<QByteArray, qreal>. -- Dmitry Shachnev
signature.asc
Description: PGP signature