ScPostIt revision

[Mike Kaganski]

Hello Eike,

thank you very much for your notes!

6/20/2015 2:44 AM, Eike Rathke пишет:
>> I think that there are only three
>> distinct types of notes out there:
> There *are* or there *should be*?

I tried to look for places where currently ScPostIt is used, and only 
found these types of usage. So, I think there are. Treating this as 
"should be" helps structuring their relations and articulate pre- and 
postconditions, so that if later another usage emerges, it would be easy 
to built it in (add the mode and specify new relations to already 
present modes) without breaking already implemented usages. If 
copy-on-write is later implemented, it would simplify things further.

>> 1. *Ordinary* objects: they belong to one and only one cell in a document.
>> They don't share anything with no other *ordinary* note, nor with
>> *clipboard* notes. But they always share data with *undo* notes.
> You'll need copy-on-write for that, otherwise modifying a note would
> also modify the Undo note.

Well, that's not a problem. In undo notes, unmodified caption state is 
not required at all times, only when it is about to be used (i.e. it's 
at the top of undo stack). Any following modification (including 
modifying the note in question) will create its own undo, and put it to 
undo stack. Rewinding modifications will bring the modified object to 
original state when the undo note in question will have to be used.

>> - to *clipboard* notes - then do deep copy; but the copy data should also be
>> able to live without drawing layer. I believe that cut-to-clipboard should
>> be treated uniformly, because there are *undo* objects that share data with
>> original object, so transferring it to clipboard is prone to problems.
> As long as they *are* the same you don't need deep copies.

Well, that *requires* the copy-on-write logic. As I mentioned, 
currently, it's not implemented for notes. And that's the major change 
that everyone advises me to avoid in the beginning. So, I don't think 
that that's what should I do now, because I do agree that major changes 
should be postponed to the moment when my understanding will be better. 
But that should be considered, because potentially that could further 
speed up the notes handling. So, as long as there's no copy-on-write, I 
suppose, what I wrote holds.

>> 2. *Clipboard* notes. They should not share anything with any other note.
>> They should be self-dependent, because otherwise (a) the clipboard contents
>> could change by changing *ordinary* notes after copy-to-clipboard was made;
> Which is why copy-on-write would be good to have.

I agree; do you think it is good time to start writing it? :)

>> and (b) clipboard should be able to live independently of original document
>> and its objects. They can only be copied to *ordinary* notes, not to *undo*
>> or other *clipboard* notes. When copying to *ordinary* note, deep copy is
>> required, because paste from clipboard can be made multiple times, so
>> independence is required.
> If you cut and paste once, no deep copy is required. Further pastes at
> that stage or copy&paste also don't require deep copies, only if one of
> the copies is modified it needs copy-on-write.

OK, let's consider that as the vector for further improvements.

>> 3. *Undo* objects: they always represent some state of one specific
>> *ordinary* note;
> Exactly.
>
>> all *undo* notes of one *ordinary* object should share its
>> data.
> If the state of the ordinary note changes it needs copy-on-write.

See above.

>> That is what I see in code, and what I suppose is sane usage of the class.
>>  From this I draw the conclusion that smart memory management in this topic
>> only depends on object *usage types*, and not on other circumstances. But I
>> may well miss something (I'm sure I do), that's why I ask to point me to the
>> usage I oversee. I want to build logic upon the stated usage modes, to make
>> most optimal memory management automatically (ScPostIt client just need to
>> specify the intended usage, and don't worry how not to mess things), not to
>> make full copies at all times.
> What is the ScPostIt client and how would it know what the "copy" will
> be used for?

A class client is any code that uses that class. If you write the code 
for undoing an operation, won't you know what you use it for? As far as 
I see, in every place where the notes are *created*, the required data 
is available (e.g., it may be directly created in methods that implement 
specific actions, or it may be called in "copy from document to 
document" methods that have both documents available to check their type 
(IsUndo()/IsClipboard())). But it's not so when they are used afterwards.

>>> I'm not saying that there is not a better way to handle note's life
>>> cycles.  As Markus pointed out, there is an unfortunate complexity in
>>> how the notes are currently handled in large part because two separate
>>> entities - Calc core and the drawing layer - claim ownership and memory
>>> management responsibility to the note objects.
> And that "two instances claim ownership" needs to be taken into account.

Agree.

> You can solve it with shared_ptr, if a proper copy-on-write is
> implemented and the copy is triggered only from Calc core and not by the
> drawing layer.

I agree that the interaction with drawing layer is very important to 
make things right. But that's incremental changes: for now, I 
concentrated on cleaning up the ScPostIt-to-SdrCaptionObj relations, 
trying to keep it safe while minimally touching external relations. That 
would (hopefully) help fix a number of bugs, without introducing more 
regressions. I hope to proceed further after that. In the meanwhile, 
making proper unit tests would help me understand the drawing layer part 
better.

-- 
Best regards,
Mike Kaganski