Cross mode integration

[nkh]

@qindapao

Open discussion about the cross mode future

Cross mode is excellent at drawing complex graphics

https://github.com/nkh/P5-App-Asciio/blob/master/documentation/mdbook_asciio/src/modes/cross_demo.gif

Cross mode is not Asciio

It uses Asciio as an environment to draw graphics.

We have discussed some points already but I think we should list them here and write a requirement about how Cross mode would work if it was integrated.

Difference with Asciio

• can't move objects as if they were connected, because they aren't
• connects anywhere not just at connection points
  • Asciio can connect anywhere but elements need to be setup accordingly
• fillers are extra elements
• fillers, I think, stops the user from selecting the arrow end to move it

Separation from Asciio

The graphics drawing mode, what we call Cross-mode today needs to be separated even more

• bindings need to go to a different file
• the application that starts it shouldn't be called Asciio, like the TUI is called Tasciio.
• code needs to be moved into another directory

Integration it into Asciio

What I believe the cross mode to be is a graphic overlay. a way to change how things look like without changing the underlying structure or adding anything.

in Asciio there should be very little difference between these renderings of the same objects.

The only differences are:

• Asciio is aware of the rendering overlay
• the elements have an extra attribute to tell the overlay they want to be drawn differently or not

New Asciio rendering pipeline

• is element part of an overlay
• is the rendering cache valid
• use cache or re-render
  • call overlay to get new strips
• display the element

optimization opportunities

• Asciio only calls the elements that are part of the overlay, overlay code can be simplified
• Asciio can keep a 2/3D map, the overlay doesn't need to compute it
  • 2D map is sparse
• Asciio can cache data generated from the overlay if necessary, simplifies the overlay

[qindapao]

@nkh

Your opinion is to the point.It is really bad that the cross mode prevents the user from selecting the intersection point.In the future, we should integrate it into the bottom of asciio.Make it a real part of asciio.

[qindapao]

Fillers are superfluous things.We need to really update the element's stripes.I haven't fully understood your other words. Give me some time to digest what you said.But I understand the central idea.Cross mode destroys the unity of software.

It's okay. We will perfect it.

I also hope that other programmers can join in the future.Maintain the consistently high quality of this interesting application.Of course, you need to control the overall situation and prevent the software architecture or style from corrupting. I trust you completely.

[nkh]

@qindapao

You coded the difficult part, now it's just integration.

I'll start a branch that calls a phony layer as an example. The strips are not replaced, they are part of the elements, their rendering is.

Once that's done I recommend a quick and dirty integration to see it working, I think it will be very easy, specially when the fillers are not needed anymore. We can optimize it later.

[qindapao]

https://github.com/nkh/P5-App-Asciio/issues/62#issuecomment-1616628981

You go on, but also pay attention to rest, my weekend is over, now I should eat my dinner. :)

[nkh]

I have other issues to fix, special keyboard handling, ... I'll find time for the overlay in the beginning of the week.

[qindapao]

@nkh

https://github.com/qindapao/P5-App-Asciio/commit/a4f5b27af68a67a426fd48de663deb7de720e576

The performance loss mainly comes from these functions:

transform_elements_to_ascii_array_for_cross_overlay characters acquisition draw_overlay function characters drawing

There is also a problem with conversion using the ascii_to_text tool, the overlay may be lost. But, We can use strip_groups to protect all the charts

[qindapao]

I don't have much time today, so I didn't have time to write the document

[nkh]

@qindapao it's not slow on my computer, Do you have a file with many elements that is slow?

Otherwise it looks good in ASCII, I'm going to test Unicode and have a look at the code.

this message was displayed: "my" variable @all_overlay_chars masks earlier declaration in same scope at /home/nadim/nadim/devel/repositories/perl_modules/P5-App-Asciio/blib/lib/App/Asciio/stripes/group.pm line 31.

[nkh]

@qindapao I think it should be all all-cross or nothing cross, I can't see a case where both should co-exist.

[nkh]

the cross mode needs to be integrated at a lower level, we must be able to show filler and single-click elements

a simple solution is to allow multiple callback, each callback needs to be uniquely identified, possibly by passing the same sub to the overlay removal function

[qindapao]

@nkh

Do you mean that an diagram is either all crossed or none crossed?

Do you mean that the definition of cross elements is redundant?

[nkh]

Yes, the whole graph is Cross type or it isn't. instead the cross state is kept in the graph. that also eliminates the problem above.

Do you have any reason to have both cross and normal elements together?

[nkh]

@qindapao

I merged your branch and set back colors to normal but I think that I found a small bug.

[qindapao]

@nkh

I have a little disagreement here,All-cross mode will really lead to serious performance degradation.The meaning of normal mode is that you don't need to participate in crossover operation and give coordinate points.Users can also easily save work results.

I modified two kinds of strip groups for this reason.What is added is just the extra strip of strip group.Without affecting the elements, these extra strips disappear after ungroup.They are just to improve performance and let users save the intermediate results of crossover.(You don't need to participate in crossover operation when you put it into strip group, which preserves the intermediate visual effect and does not affect the export.)

I agree to set the cross element color back to normal.But please keep the cross elements.

It is also possible that users want some elements to cross and some elements not to cross.Can we keep this flexibility?

[qindapao]

@nkh

The above one is not a bug, it is the lack of such characters.I wrote a todo in the code. The mixed symbols of double lines and thin lines are limited, and bold lines and thin lines have such characters.

But there are too many bold and thin mix characters (45), So I haven't implemented them yet

[nkh]

@qindapao I'm going to disagree with you about performance, we need to set the bar high and take measurements, when you're done documenting we have to take another round of refactoring, there are a few things I believe can be improved, including some code that should be moved around.

[qindapao]

@nkh

I can do as you say. I have a compromise.Let me take a moment to explain.

[nkh]

And I have at least two ways to optimize the overlay generation, you explain first :)

[qindapao]

@nkh

I will ignore two kinds of strip groups in the function that gives coordinates.

So that the overlay does not perceive them. It doesn't matter if there are all cross elements outside.

Strip groups still retain the characters of the current overlay, which I call visual effect retention.

Can you accept this?

Giving coordinates is a user behavior, which can be customized by users.The only change is that strip groups keeps the current overlay characters, but does not destroy the elements.I explained it above.

Strip groups means freeze.I understand that freezing the current visual state is it.

[qindapao]

@nkh

It should be very late at your place. Take a rest first. We will discuss these detail slowly.

[nkh]

If you mean that a strip-group should not be part of a cross drawing then I am split about that decision. We don't have to take a decision now and by default they could be non-crossing if you want and the user can decide if that they are crossing.

1-strip-groups were invented for you and I don't think anyone will ever push Asciio to its limits like you do, so I'll let the decision to you but I made a video explaining how I think, and I also demo the use of connectors.

diff_group_1_strip.asciio.gz

[qindapao]

@nkh

I understand the meaning of your animation, You have demonstrated this usage before, and I know it.

OK, You convinced me. I'll submit the code again later.

The performance problem is left to performance optimization.It has nothing to do with crossing.

We can end this discussion.Regarding the performance under cross, I still hope to improve it a little.

[qindapao]

@nkh

You mean that if the strip group does not cross, it will lead to disunity with ordinary groups or other elements.

[nkh]

@qindapao yes, the strip group and the normal group do not behave in the same way in cross mode. As I wrote above I don't use strip-groups.

I think that the best way is to remove the strip-group from consideration based on an option. That option can be off by default so you get the behavior you want, users that want another behavior can change it.

[qindapao]

@nkh

I only need a simple way to judge whether elements overlap, which can greatly improve efficiency.

[qindapao]

@nkh

That's fine. I'll do it. Cheers. We finally agree.

[nkh]

Tell me a bit more about the overlap information you need, is it a position, a list of the elements, their depth, a 2D maps?

I'd like to:

• ask you for a graph that shows the problem with efficiency
• help you refactor the Cross code, not coding just telling you were, some things are not placed where they should, I'll open individual issues (btw you're also welcome to review what I do)
• continue talking about how to optimize in this issue

[qindapao]

@nkh I will provide you with the efficiency chart after the next commit.

For example, if there is an element, I want to know whether it is an element covered by other elements, or whether it covers others. What I want now is this.

It would be better if i can quickly know the character list corresponding to each coordinate without exporting to ascii.

[qindapao]

@nkh

If it is an element without any coverage, I can just ignore it and not read its strip information.

[qindapao]

@nkh You have added a lot of things recently, and I still have to take time to digest them.

[nkh]

I'd rather have the cross mode global and exclude some elements (like strip-groups, we can put that in the config instead for a simple boolean), so there is no need to add CROSS_FLAG to elements, instead it becomes an option in Asciio.

that also completely eliminates the Cross popup-menu, all objects are cross-type, we just run the cross-mode code depending on the Asciio option.

• all objects "normal', no CROSS_FLAG
• when cross-mode is enable the cross code is called, I'd like to integrate that more so it has it doesn't even need to use the overlay mechanism, it will work the same, I can code that or show you how
• if the cross-mode is enables, it filters out the strip-groups if configured to do so.

[qindapao]

@nkh

Very good, but does it mean a big change?

[qindapao]

@nkh Global options turn crossing on or off, and add special element exclusion functions, such as ellipses, strip groups, and so on.

I like this plan.

[nkh]

I think it's a small change.

on your side:

• let's merge what you have now, even if it's small changes
• create a new branch for the full integration
• remove all the CROSS_FLAGS from all objects and code, and configs
• create a function that can be called in the rendering module
  • it return overlay "objects" just like before, we could integrate it more but we would pollute your module with GTK
  • It can use the $asciio argument to check what type of elements to discard
    • the option in the config should be a list of ll the discarded types, not a boolean, example below
• remove all the bindings pertaining to cross-mode
• create a new binding that sets the Asciio USE_CROSS_MODE config
• remove the popup-menu entry

so most of it is removal and simplification so it's quite easy

the call to is from the rendering module will be something like

my @overlays = cross_mode_get_overlays($asciio) if $asciio->{USE_CROSS_MODE} ; 

the config should look like:

USE_CROSS_MODE => 0,
CROSS_MODE_IGNORE =>
    [
    'class to ignore',
    'other class to ignore',
   ...
   ],

...

[qindapao]

@nkh I don't have any new code at present, So I do the full integration,It takes a little time.

[nkh]

Take your time, I'm working on the "dot" functionality and element cloning with the mouse.

namespace

module Cross.pm in in package App::Asciio, please change that to App::Asciio::Cross

entry point sub, it's the callback

App::Asciio::get_cross_points_coordinates

which should become

get_cross_mode_overlays

bindings

remove the whole section

'Cross element Insert leader' => 
    {
    SHORTCUTS => '000-x',

    'Add cross box'                       => ['000-b', \&App::Asciio::Actions::Elements::add_element, ['Asciio/Cross/box', 0]                 ],
    'Add cross exec box'                  => ['000-e', \&App::Asciio::Actions::Elements::add_element, ['Asciio/Cross/exec box', 1]            ],
    'Add cross arrow'                     => ['000-a', \&App::Asciio::Actions::Elements::add_element, ['Asciio/Cross/wirl_arrow', 0]          ],
    'Add cross angled arrow'              => ['00S-A', \&App::Asciio::Actions::Elements::add_element, ['Asciio/Cross/angled_arrow', 0]        ],

    'add cross ascii line'                => ['000-w', \&App::Asciio::Actions::Elements::create_line, [0, 1]                                  ], 
    'add cross unicode line'              => ['00S-W', \&App::Asciio::Actions::Elements::create_line, [1, 1]                                  ],
    'Add cross unicode bold line'         => ['C00-w', \&App::Asciio::Actions::Elements::create_line, [2, 1]                                  ],
    'Add cross unicode double line'       => ['0A0-w', \&App::Asciio::Actions::Elements::create_line, [3, 1]                                  ],

    'Select cross elements'               => ['000-c', \&App::Asciio::Actions::ElementsManipulation::select_cross_elements_from_selected_elements               ],
    'Select normal elements'              => ['000-n', \&App::Asciio::Actions::ElementsManipulation::select_normal_elements_from_selected_elements              ],

    'change to cross elements'            => ['C00-c', \&App::Asciio::Actions::ElementsManipulation::switch_to_cross_elements_from_selected_elements            ],
    'change to normal elements'           => ['C00-n', \&App::Asciio::Actions::ElementsManipulation::switch_to_normal_elements_from_selected_elements           ],
    },

remove

'set cross overlays'   => [ '0A0-o', sub { $_[0]->set_overlays_sub(\&App::Asciio::get_cross_points_coordinates) ; $_[0]->update_display ; } ],
'reset overlays' => [ '000-o', sub { $_[0]->set_overlays_sub(undef) ; $_[0]->update_display ; } ],

add

'flip cross mode'   => [ 'shortcut', sub { $_[0]->{USE_CROSS_MODE) ^= 1 ; $_[0]->update_display ; } ],

shortcut

000-x is free, and it makes cross mode easy to acces, a first class citizen

[qindapao]

@nkh

https://github.com/qindapao/P5-App-Asciio/commit/f0785da83f17b95296a7b5e5c3a3b1cd29d9f06f

i have done my part

[nkh]

@qindapao Great work! I'll integrate it in a few hours.

Once that's done I'll do a review I noticed some code that' in the wrong package but maybe you have already fixed it.

[qindapao]

@nkh

I found a few places that I forgot to change, but it doesn't matter, it won't affect your work. I'll fix it when you're finished. It's just a few small bugs.

I'm off to bed now.

[nkh]

Hi, I couldn't find time for integration because I've hit a major problem and I have Asciio crashing, some memory management problem with GTK objects, it's a headache. I will work on it today.

[qindapao]

@nkh

Take your time. After the module is replaced by App::Asciio::Cross, I expect to run the code according to the way overlay works, but it reported some errors that I can't understand.

[qindapao]

@nkh

You'd better have a good rest first

The problem I encountered was that the function in @normal_char_func could not be called normally at all.

[nkh]

Can you please list here the error messages you get, what you expect to happen and what happens, what you write above is not enough for me to help.

[qindapao]

@nkh

I can only use my mobile phone now, and I'll send it to you by computer when I get home at night.

[qindapao]

@nkh

I don't know if you are asleep.

My problem is solved. I forgot these.

use List::Util qw(first) ; use List::MoreUtils qw(any);

I think I should sum up, so as not to waste time on such low-level issues.

Cross.pm Now these two lines must be added to work normally, and I will give a patch at night.

[nkh]

@qindapao I fixed the new "clone" functionality and I'm going to merge your work now. Then I'll do a review to find the places where the Cross code is but shouldn't be. There's almost no reason to have cross code in any other place than cross.pm

It would be good if we could have an interactive session.

[qindapao]

@nkh

It's a pity that I'm still in the company.

use List::Util qw(first) ; use List::MoreUtils qw(any);

These two sentences should be added to cross.pm

Leave the unnecessary unreasonable code to me to deal with.

[qindapao]

@nkh

The problem of your GTK crash is solved？

[qindapao]

@nkh

Exporting to ascii needs to determine whether it is in cross mode.

[nkh]

Crash solved, references to references to references to GTK objects that get Clones and crash, a headache to find.

I have made a video of the new clone mode, I'll push it in a few minutes.

I think we can fix the exporting to ASCII in a better way, we need to hunt down those little pieces of code and move them to Cross.pm.