Hi Markus,
Good to see you again after a long hiatus. :-)
On 8/10/25 16:55, Markus Mohrhard wrote:
Attached are two screenshots showing a comparison between the MSO
Excel rendering of my test document and the current Calc rendering.
I think the rendering looks decent, even without font handling.
* font formatting is not implemented at all. Font handling is a lot
more complicated than background and border handling
Indeed. I just looked through the code in question, and the complexity
is quite high there. Lots of special case handling...
* unit tests are completely missing
I don't think that's a big deal especially for rendering features.
* link between table style and DB goes through the name instead of a
reference to the actual table definition
I think that's fine as well. If we decide later that there is a better
way we can switch to that.
* The autoformat feature should be removed and folded into the table
style implementation which would also provide a UI for modifying
table styles.
Auto format is such an outdated concept that I don't think anyone would
miss it. Now, a UI for modifying table styles, that can be done in a
separate step at least initially especially when availability of time
(your time) is limited.
* merged cells are not handled at all right now
A corner case that can be handled later, if it's really needed.
* some UNO interfaces to interact with this feature
... can definitely wait. :-)
Additionally, I might fix the following database range related issues
before merging if I continue with the current design:
No objections from me.
Now to the part that I'm not happy with and why I think someone else
might want to have a second look at the ideas behind this change:
* Storing the table style information purely on the ScDBData makes
some operations quite easy but at the same time makes the code in
ScDocument::FillInfo a lot more complicated. At the same time I
have not come up with a design that would allow us to avoid the
position dependent property lookup during FillInfo. We could store
a reference to the table style in the ScPatternAttr but then we
need to keep the ScDBData range and the ScPatternAttr in sync. In
my mind the table style information is not a cell attribute until
the rendering stage.
Table style information IMO is definitely not a cell attribute as you
say. In terms of how to store the table style information, I don't have
a strong opinion, but maybe you can try storing it outside of ScDBData
and use ScDBData's name as a way to reference it. I would personally
avoid touching ScPatternAttr for this feature, unless you absolutely
have to.
* Adding font handling is going to be painful as the font lookup is
delayed until the actual drawing
o My current idea would be to store a reference to the table
style and a struct storing the style lookup information, e.g.
this cell looks at first column stripe, second row stripe and
whole table item sets. This way the information only needs to
be computed once and the lookup for all the font properties
can be done reasonably quickly.
o Potentially this could also be done by combining the handling
of conditional formatting and table styles. In theory a
slightly cleaner approach would be to have a list of
SfxItemSet instances that are checked in order until an
explicitly set item was found. That way there is no need to
encode application layer information into the rendering code.
Sounds sensible to me. Conditional formatting is probably the closest
feature in terms of applying extra font attributes, so it makes sense to
keep the new logic close to it.
* The fix for the border drawing issue mentioned above requires the
dynamic generation of new border items that can be passed through
ScDocument::FillInfo to the rendering code or a rework of the
border rendering.
I think FillInfo itself could use some rework. ;-) It's screaming for
rework every time I look at it... Way too much logic is lumped into one
function body... But honestly I haven't spent that much time struggling
with this part of the Calc code, so take this with a grain of salt.
I'd appreciate feedback from some other Calc developers about the
design and whether it is worth continuing with this design.
I think it's well worth pursuing, if you are willing.
My 2 cents.
Kohei
