nkh / P5-App-Asciio

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

Cross mode error near boundaries #130

Closed nkh closed 8 months ago

nkh commented 10 months ago

If a crossing is close to the boundary when the window is scrolled, the wrong overlay is computed.

screenshot_2023-10-05_09-58-32

screenshot_2023-10-05_09-58-47

I also noticed that the code below, from GTK::Asciio, passed the boundaries in pixels, shouldn't it be in characters?

my ($start_x, $end_x, $start_y, $end_y) = 
    (
    int( $h_value / $character_width ), 
    int( ($h_value + $windows_width) / $character_width ),
    int( $v_value / $character_height),
    int( ($v_value + $windows_height) / $character_height)
    ) ;

for (App::Asciio::Cross::get_cross_mode_overlays($self, $start_x, $end_x, $start_y, $end_y))
    {
    my ($x, $y, $overlay, $background_color, $foreground_color) = @$_ ;
qindapao commented 10 months ago

@nkh I know about the problem that the boundaries cannot be displayed at crossing. It is just for display, because the characters outside the boundaries are not considered when I display the interface. If I want to solve this problem, I can just take one more character in four directions. This issue does not affect exported data.

As for whether to use pixels or characters to determine boundaries, I’ll think about it again, maybe a week later.

qindapao commented 10 months ago

@nkh

I solved this problem, here is the branch to solve the problem. At the same time, the cross-mode refactor has probably been verified and can be integrated first. If there are minor problems later, Let me fix it again.

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

I condensed all the fixes into one commit since they are all related to cross-mode.

cross_boarder_fix

nkh commented 10 months ago

That branch doesn't fix the problem, here's a better description of how to reproduce the error.

screenshot_2023-10-06_13-25-02

qindapao commented 10 months ago

@nkh I tried, and now it can be displayed normally when the scroll bar is not at the boundary.

but When the scroll bar is at the left boundary and the upper boundary, because the index of the characters on the left and above the cross point is less than 0, it is ignored when exporting to ascii. can't put it in an array. It does cause a cross character exception.

If the characters next to the cross point are beyond the boundary, then the display of the cross point is meaningless.

If we want to solve the left boundary and upper boundary problems. We can't use the array index to correspond to the character position, because -1 as the array index has a special meaning, which may be too big to be worth it.

I have no good ideas for the time being.

nkh commented 10 months ago

I think it is worth it since I encountered this problem twice this week during normal utilization.

Can you send me link to the code where -1 is used as a special case? maybe we need to use something else.

nkh commented 10 months ago

IMO the crossing code should not have access to the whole array, all it needs is two characters plus the neighbors. the code to generate the neighbors map, 8 cells, could handle this case.

qindapao commented 10 months ago

@nkh I won't be able to use my computer for five days. I'm browsing on my mobile phone now. When we export characters, we use a two-dimensional array to record every character in the 2D plane. -1 can't be used in a two-dimensional array.

nkh commented 10 months ago

please send me a link to where we index the array

qindapao commented 10 months ago

Negative index is to take elements backwards, which is not what we want.

qindapao commented 10 months ago

get_ascii_array_and_crossings

And the current situation is that characters with coordinates less than 0 are ignored by us.

qindapao commented 10 months ago

I can't send the exact link with my mobile phone, so I can only give the function name.

nkh commented 10 months ago

Function name was fine.

There are two elements to consider here.

usage of -1

that's because, if I get it right, you build the 3D @lines in reverse, IE the elements in asciio are deepest first as it's the drawing order, but when you build @line they end up in the front of @lines and to get the element that in the front in the display you use -1 to get the front most element.

that's easily fixed

# replace 
push @{$lines[$y][$x]}, $character 

#with 
unshift @{$lines[$y][$x]}, $character 

we now have the foremost element at position 0, thus removing the special usage of -1

But that's not the problem with -1!

$ascii_array[$row][$col][-1]

it's when $row or $col is -1 that we encounter the problem

detecting out of bounds characters

Even if we detect that $row or $col are equal to -1, we can't get the neighboring character because it never made it into the 2D map because of this code.

next if defined $start_y && ($y < $start_y || $y >= $end_y) ;

which takes us back to "Refactor: transform_elements_to_ascii_two_dimensional_array" and this comment https://github.com/nkh/P5-App-Asciio/issues/108#issuecomment-1748507547

This issue can be resolved with the way we build the 2D map today but is easily fixed when we have a Z buffer; I'll try to give it a higher priority.

qindapao commented 10 months ago

next if defined $start_y && ($y < $start_y || $y >= $end_y) ;

@nkh Here I have subtracted 2 from starty, and then added 2 to end y.This is no longer a problem.I also relaxed the upper and lower limits by 2 in the X coordinate.

elsif($x >= 0 && $y >= 0) {

keep the characters that may be crossing in the array

                # other characters are discarded
                if(exists $cross_filler_chars->{$character})
                    {

The problem here is that the elements with negative coordinates are not in the 2-D array at all.

$ascii_array[$row][$col][-1]

There is nothing wrong with this code.Because row and col will not appear negative numbers, the previous judgment prevents negative numbers.

This is only to determine whether the topest character is the expected character. If it is, there is no need to generate a cross character.

The existing code has been difficult to solve.Maybe, as you said, a new scheme is needed, but there may be some details to pay attention to.

qindapao commented 10 months ago

@nkh I still hope that there will not be too much structural adjustment in 1.9.Let's stabilize it first.

qindapao commented 10 months ago

@nkh Under the existing code, it may be possible to take the characters at the -1 coordinate as 0 and move all the characters to the right.

The ordinate is also treated in the same way

I think it may be the fastest way to solve this problem at present.

qindapao commented 8 months ago

@nkh Hello, I verified that this problem should have been solved.

nkh commented 8 months ago

@qindapao in which commit was it solved?

qindapao commented 8 months ago

@nkh d426f93

My computer is off, but I just verified that there is no problem. Please confirm it again.