nkh / P5-App-Asciio

Plain ASCII diagram
https://nkh.github.io/P5-App-Asciio/
56 stars 4 forks source link

Integrate cross mode using zbuffer #165

Closed qindapao closed 1 year ago

qindapao commented 1 year ago

That post is too long, I will create a new one

@nkh I've basically finished writing it, but there are still some small adjustments to the zbuffer module. Please let me explain it clearly before you get confused.

https://github.com/qindapao/P5-App-Asciio/tree/cross_integration

qindapao commented 1 year ago

@nkh

(1). $character_index += unicode_length($char);

Coordinates may contain double-width characters (such as Chinese) and zero-width characters (Arabic, etc.)

(2). if(is_nonspacing_char($char))

0-width characters are appended to the previous character,

(3). $strip = $USE_MARKUP_CLASS->delete_markup_characters($strip) ;

The markup character needs to be deleted. Of course, this should not appear here. I will get rid of it later when I reconstruct the markup module.

nkh commented 1 year ago

@qindapao

(1) ok

(2) please explain why this is needed

(3) ok till we Markup is refactored

(4) the zbuffer that's send to get_cross_mode_overlay is READ ONLY but you still modify it

You make a copy of it in $intersecting_element but still modify the zbuffer, please fix, IE: modify your copy.

Also, I don't think cloning the whole intersections is necessary; we can have a look at that once you re-write your code to use you copy and not Zbuffer.

my ($zbuffer, $start_x, $end_x, $start_y, $end_y) = @_;

my $intersecting_elements = Clone::clone($zbuffer->{intersecting_elements}) ;

while (my ($coordinate, $char_stacks) = each %$intersecting_elements)
        ...
    if($char_count >= 2)
        {
        $zbuffer->{intersecting_elements}{$coordinate} = $cross_array ;
                ^^^^^^^ is read only, use your copy
        }

I tried it and it works fine, I'll merge once you have made the changes above.

qindapao commented 1 year ago

@nkh That's because there are more cross points than I want. I did a second filtration.

Can I modify the clone object?

I currently modified the original object.

At present, there are more intersections and characters than I want.

qindapao commented 1 year ago

(2) Because you will use zbuffer for rendering, if you lose the nospacing character. Arabic will display an exception, and the combined characters are lost.

qindapao commented 1 year ago

@nkh You agree with me to modify my copied object, right?Because I want to use the get neighbor function of zbuffer module, I may clone the whole zbuffer object. Will this be time-consuming?

nkh commented 1 year ago

@qindapao your object is yours to do what you want with it. I think it's not necessary and takes time but we can start with that an optimize later.

Copying the whole object or copying only the intersections take the same time.

qindapao commented 1 year ago

@nkh All right. I'll fix it tomorrow.

qindapao commented 1 year ago

@nkh There is another problem that I forgot. Under the current architecture, the variable CROSS_MODE_IGNORE has no effect.I can't filter any elements.If that's the case, I'll have to update the document about cross mode tomorrow. It also includes the part of crossover algorithm that is not limited to two characters.. So as not to mislead users and future developers.

nkh commented 1 year ago

@qindapao I asked you earlier why we were filtering triangle and ellipses.

qindapao commented 1 year ago

@nkh I can't remember the specific reason, maybe it's performance considerationsI'll cancel it first.At that time, it was possible to consider that the characters of the ellipses could not cross.Rule it out to save time.Now that it is fast enough, this strategy is irrelevant.The removal reduce that complexity of the user.

qindapao commented 1 year ago

@nkh

https://github.com/qindapao/P5-App-Asciio/tree/cross_integration

I haven’t started with transform_elements_to_characters_array yet, so let’s do it step by step.

nkh commented 1 year ago

@qindapao can you please squash your two commits and use the shortest commit message.

I've just committed 097f80c which implements #160, you may need to rebase too.

qindapao commented 1 year ago

@nkh

https://github.com/qindapao/P5-App-Asciio/tree/cross

I squash it and this is the new branch

nkh commented 1 year ago

@qindapao merged into main branch

Can you please delete the old branches, in your repo, that you are not using anymore?

screenshot_2023-11-25_15-37-33

qindapao commented 1 year ago

@nkh Ok, I see. I'll deal with it.