Looking good.

Do you mean this "if ()" part of code? If there is no style "graphic" and inner "if" will not execute. Do you mean it'll be better to handle it in this part of code...or what?

loadTableCellProperties() is a method for applying styles for table cells, i.e. cell background. There is no handling for cell background in the place you mentioned.

Don't know this shape stuff that well, but text shape plugin handling custom-shape feels suspicious.

Not sure about all the details but two things look suspicious here:

Actually seems to now revert that one line as it was before commit fc7b878f206fb / https://bugs.kde.org/show_bug.cgi?id=306141

Seems to work and big impact indeed. Approving already.

Or in other words: support for escaped brackets \{ \}

A bit mixed feelings for this. In a way nice to avoid unnecessary work, but then again it seems effectively a compilation option if xlsx support is for read-write or view-only. On main level CMake it's also a bit random detail compared to others.

Seems reasonable to me. I'll approve later if no-one objects.

Seems reasonable to me.

But would those characters work without encoding adjustments if the used MS font was present? Not sure how it gets rendered now without the font, but if it's anything like the "J" exchange email smiley I'm not sure which is worse.

Is there something in the spec encoding the microsoft fonts or just some kind of practice? Seems a little scary just stripping bits.

Minor comment, but overall looks good to me.

Seems sensical for what I can tell. I'll do some test round and (likely) approve later if no objections pop up.

Seems sensible to me. I'll approve and merge shortly if no-one objects.

Seems good.

Seems good.

Looks good to me.

Haven't looked and to be honest a little skeptical if anyone removing a null check is likely enough to protect against.

Looks good to me.

Also remove the empty target file on error

Update image size reading conditions

Whitespace adjustments

Seems it was indeed the only physicalDpi, though not too many logical either. Not trivial to follow, but seems sane and the screenshots are thorough. I'll approve if no-one objects shortly.

Looking good to me.

And even rendering now better, no idea what happened.

Having a bit strange rendering for TextDocumentImpl.cpp changes here, but the raw diff looks good now. Trust all good :)

Indeed that doesn't look like a good loop, if triggered once, nothing affecting the break out condition inside of it. But isn't the same thing happening also at the beginning of this method, and actually same kind of construct being there in spreadsheet and presentation impls too?

Second ping for this.