<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Hi Markus,</p>
<p>Good to see you again after a long hiatus. :-)<br>
</p>
<div class="moz-cite-prefix">On 8/10/25 16:55, Markus Mohrhard
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAEDdEt6R-XhrT85V81kWJ9GBvs=O39-y+hP2dhMAMgw9u+vzLQ@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div dir="ltr">
<div>Attached are two screenshots showing a comparison between
the MSO Excel rendering of my test document and the current
Calc rendering.</div>
</div>
</blockquote>
I think the rendering looks decent, even without font handling.
<blockquote type="cite"
cite="mid:CAEDdEt6R-XhrT85V81kWJ9GBvs=O39-y+hP2dhMAMgw9u+vzLQ@mail.gmail.com">
<div dir="ltr">
<div>
<ul>
<li>font formatting is not implemented at all. Font handling
is a lot more complicated than background and border
handling</li>
</ul>
</div>
</div>
</blockquote>
<p>Indeed. I just looked through the code in question, and the
complexity is quite high there. Lots of special case handling...<br>
</p>
<blockquote type="cite"
cite="mid:CAEDdEt6R-XhrT85V81kWJ9GBvs=O39-y+hP2dhMAMgw9u+vzLQ@mail.gmail.com">
<div dir="ltr">
<div>
<ul>
<li>unit tests are completely missing<br>
</li>
</ul>
</div>
</div>
</blockquote>
I don't think that's a big deal especially for rendering features.<br>
<blockquote type="cite"
cite="mid:CAEDdEt6R-XhrT85V81kWJ9GBvs=O39-y+hP2dhMAMgw9u+vzLQ@mail.gmail.com">
<div dir="ltr">
<div>
<ul>
<li>link between table style and DB goes through the name
instead of a reference to the actual table definition</li>
</ul>
</div>
</div>
</blockquote>
I think that's fine as well. If we decide later that there is a
better way we can switch to that.<br>
<blockquote type="cite"
cite="mid:CAEDdEt6R-XhrT85V81kWJ9GBvs=O39-y+hP2dhMAMgw9u+vzLQ@mail.gmail.com">
<div dir="ltr">
<div>
<ul>
<li>The autoformat feature should be removed and folded into
the table style implementation which would also provide a
UI for modifying table styles.</li>
</ul>
</div>
</div>
</blockquote>
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.<br>
<blockquote type="cite"
cite="mid:CAEDdEt6R-XhrT85V81kWJ9GBvs=O39-y+hP2dhMAMgw9u+vzLQ@mail.gmail.com">
<div dir="ltr">
<div>
<ul>
<li>merged cells are not handled at all right now</li>
</ul>
</div>
</div>
</blockquote>
A corner case that can be handled later, if it's really needed.<br>
<blockquote type="cite"
cite="mid:CAEDdEt6R-XhrT85V81kWJ9GBvs=O39-y+hP2dhMAMgw9u+vzLQ@mail.gmail.com">
<div dir="ltr">
<div>
<ul>
<li>some UNO interfaces to interact with this feature<br>
</li>
</ul>
</div>
</div>
</blockquote>
... can definitely wait. :-)<br>
<blockquote type="cite"
cite="mid:CAEDdEt6R-XhrT85V81kWJ9GBvs=O39-y+hP2dhMAMgw9u+vzLQ@mail.gmail.com">
<div dir="ltr">
<div>
<div><br>
</div>
<div>Additionally, I might fix the following database range
related issues before merging if I continue with the current
design:</div>
</div>
</div>
</blockquote>
No objections from me.
<blockquote type="cite"
cite="mid:CAEDdEt6R-XhrT85V81kWJ9GBvs=O39-y+hP2dhMAMgw9u+vzLQ@mail.gmail.com">
<div dir="ltr">
<div>
<div>
<div>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:</div>
<div><br>
</div>
<div>
<ul>
<li>Storing the table style information purely on the
ScDBData makes some operations quite easy but at the
same time makes the code in <a class="moz-txt-link-freetext" href="ScDocument::FillInfo">ScDocument::FillInfo</a> 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.<br>
</li>
</ul>
</div>
</div>
</div>
</div>
</blockquote>
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.<br>
<blockquote type="cite"
cite="mid:CAEDdEt6R-XhrT85V81kWJ9GBvs=O39-y+hP2dhMAMgw9u+vzLQ@mail.gmail.com">
<div dir="ltr">
<div>
<div>
<div>
<ul>
<li>Adding font handling is going to be painful as the
font lookup is delayed until the actual drawing</li>
<ul>
<li>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.</li>
<li>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.<br>
</li>
</ul>
</ul>
</div>
</div>
</div>
</div>
</blockquote>
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.<br>
<blockquote type="cite"
cite="mid:CAEDdEt6R-XhrT85V81kWJ9GBvs=O39-y+hP2dhMAMgw9u+vzLQ@mail.gmail.com">
<div dir="ltr">
<div>
<div>
<div>
<ul>
<li>The fix for the border drawing issue mentioned above
requires the dynamic generation of new border items
that can be passed through <a class="moz-txt-link-freetext" href="ScDocument::FillInfo">ScDocument::FillInfo</a> to the
rendering code or a rework of the border rendering.</li>
</ul>
</div>
</div>
</div>
</div>
</blockquote>
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.<br>
<blockquote type="cite"
cite="mid:CAEDdEt6R-XhrT85V81kWJ9GBvs=O39-y+hP2dhMAMgw9u+vzLQ@mail.gmail.com">
<div dir="ltr">
<div>
<div>
<div>I'd appreciate feedback from some other Calc developers
about the design and whether it is worth continuing with
this design.</div>
</div>
</div>
</div>
</blockquote>
<p>I think it's well worth pursuing, if you are willing.</p>
<p>My 2 cents.<br>
</p>
Kohei<br>
</body>
</html>