Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

statepixel PoC runs incorrectly on kitty #1704

Closed
dankamongmen opened this issue Jun 1, 2021 · 11 comments · Fixed by #1846
Closed

statepixel PoC runs incorrectly on kitty #1704

dankamongmen opened this issue Jun 1, 2021 · 11 comments · Fixed by #1846
Assignees
Labels
bitmaps bitmapped graphics (sixel, kitty, mmap) bug Something isn't working
Milestone

Comments

@dankamongmen
Copy link
Owner

The statepixel PoC ought load an image, and then iterate over it with a 1x1 white plane on top. This is intended to exercise scrubbing and rebuilding. It works correctly in sixel on XTerm, but in kitty, we both get flicker, and residue on the cells the 1x1 plane has crossed:

2021-06-01-030613_1072x1417_scrot

@dankamongmen dankamongmen added bug Something isn't working bitmaps bitmapped graphics (sixel, kitty, mmap) labels Jun 1, 2021
@dankamongmen dankamongmen added this to the 3.0.0 milestone Jun 1, 2021
@dankamongmen dankamongmen self-assigned this Jun 1, 2021
@dankamongmen
Copy link
Owner Author

2021-06-01-030645_1072x1417_scrot

@dankamongmen
Copy link
Owner Author

these two are composed atop one another

@dankamongmen
Copy link
Owner Author

so yeah, we're keeping a big white panel around somehow

@dankamongmen
Copy link
Owner Author

and yet the debug output reliably shows a 1x1 plane:

 -------------------------- notcurses debug state -----------------------------
  *************************   0x561dd3573850 pile ****************************
0000 off y:   6 x:  85 geom y:   1 x:   1 curs y:   0 x:   0 0x561dd35add80 
 bound 0x561dd35adaa00x561dd35adb10 → (nil) binds (nil)
0001 off y:   0 x:   0 geom y:  24 x:  86 curs y:   0 x:   0 0x561dd35adaa0 bmap
 bound 0x561dd35737700x561dd35737e0 → (nil) binds 0x561dd35add80
0002 off y:   0 x:   0 geom y:  70 x: 107 curs y:   0 x:   0 0x561dd3573770 std
 bound 0x561dd35737700x561dd3573860 → (nil) binds 0x561dd35adaa0
 ______________________________________________________________________________
 -------------------------- notcurses debug state -----------------------------
  *************************   0x561dd3573850 pile ****************************
0000 off y:   7 x:  85 geom y:   1 x:   1 curs y:   0 x:   0 0x561dd35add80 
 bound 0x561dd35adaa00x561dd35adb10 → (nil) binds (nil)
0001 off y:   0 x:   0 geom y:  24 x:  86 curs y:   0 x:   0 0x561dd35adaa0 bmap
 bound 0x561dd35737700x561dd35737e0 → (nil) binds 0x561dd35add80
0002 off y:   0 x:   0 geom y:  70 x: 107 curs y:   0 x:   0 0x561dd3573770 std
 bound 0x561dd35737700x561dd3573860 → (nil) binds 0x561dd35adaa0
 ______________________________________________________________________________
 -------------------------- notcurses debug state -----------------------------
  *************************   0x561dd3573850 pile ****************************
0000 off y:   8 x:  85 geom y:   1 x:   1 curs y:   0 x:   0 0x561dd35add80 
 bound 0x561dd35adaa00x561dd35adb10 → (nil) binds (nil)
0001 off y:   0 x:   0 geom y:  24 x:  86 curs y:   0 x:   0 0x561dd35adaa0 bmap
 bound 0x561dd35737700x561dd35737e0 → (nil) binds 0x561dd35add80
0002 off y:   0 x:   0 geom y:  70 x: 107 curs y:   0 x:   0 0x561dd3573770 std
 bound 0x561dd35737700x561dd3573860 → (nil) binds 0x561dd35adaa0
 ______________________________________________________________________________

hrmmmm what could it be? i guess we're not redrawing the cell because it's obscured by the bitmap once the 1x1 plane leaves? then we would expect a transparent one not to suffer this problem. and indeed we do not see it on atma.png....even though it seems we ought see it on some cells?

@dankamongmen
Copy link
Owner Author

there are two issues here:

  • flicker caused by rapidly deleting and redrawing the bitmap (each move of the white cell involves a wipe and a restore, requiring one redraw)
  • the white cell being left behind in kitty

the first will be resolved by #1439, which suddenly becomes much more important. this bug is about the latter. capturing the output confirms we're not clearing out the white cells, so it ought just be a matter of ensuring they're damaged.

@dankamongmen
Copy link
Owner Author

yeah, confirmed that we're not emitting the cleanup glyph:

damaging due to wipe [ ] 0/0                                                                               
damaged due to opaque else 0 0                                                                             
RAST 00000020 [ ] to 0/0 cols: 1 40ffffff40ffffff                                                          
damaging due to wipe [ ] 0/1                                                                               
damaged due to opaque else 0 1                                                                             
RAST 00000020 [ ] to 0/1 cols: 1 40ffffff40ffffff                                                          
damaging due to wipe [ ] 0/2                                                                               
damaged due to opaque else 0 2                                                                             
RAST 00000020 [ ] to 0/2 cols: 1 40ffffff40ffffff                                                          
damaging due to wipe [ ] 0/3                                                                               
damaged due to opaque else 0 3                                                                             
RAST 00000020 [ ] to 0/3 cols: 1 40ffffff40ffffff   

@dankamongmen
Copy link
Owner Author

those two "damaged" are both coming out of the same rendering cycle, so we're definitely not emitting the replacement glyph. this is probably only working in sixel because there we blow away the underlying glyph when we redraw. i bet that on a transparent bitmap, it wouldn't work there, either.

@dankamongmen
Copy link
Owner Author

nope, it works with a transparent for both sixel and kitty, and indeed we see the glyph emission:

pile 0x560cc09d3850 ymax: 70 xmax: 107                                                                     
damaging due to wipe [ ] 0/0                                                                               
damaged due to opaque else 0 0                                                                             
pile 0x560cc09d3850 ymax: 70 xmax: 107                                                                     
RAST 00000020 [ ] to 0/0 cols: 1 40ffffff40ffffff                                                          
pile 0x560cc09d3850 ymax: 70 xmax: 107                                                                     
damaging due to wipe [ ] 0/1                                                                               
damaged due to opaque                                                                                      
damaged due to opaque else 0 1                                                                             
pile 0x560cc09d3850 ymax: 70 xmax: 107                                                                     
RAST 00000000 [] to 0/0 cols: 0 0000000000000000                                                           
RAST 00000020 [ ] to 0/1 cols: 1 40ffffff40ffffff                                                          
damaging due to wipe [ ] 0/2                                                                               
damaged due to opaque                                                                                      
damaged due to opaque else 0 2                                                                             
pile 0x560cc09d3850 ymax: 70 xmax: 107                                                                     
RAST 00000000 [] to 0/1 cols: 0 0000000000000000                                                           
RAST 00000020 [ ] to 0/2 cols: 1 40ffffff40ffffff                                                          
damaging due to wipe [ ] 0/3                                                                               
damaged due to opaque                                                                                      
damaged due to opaque else 0 3                                                                             
pile 0x560cc09d3850 ymax: 70 xmax: 107                                                                     
RAST 00000000 [] to 0/2 cols: 0 0000000000000000                                                           
RAST 00000020 [ ] to 0/3 cols: 1 40ffffff40ffffff   

@dankamongmen
Copy link
Owner Author

yep, needed to damage in the rebuild conditional. let's test this, but it works for this issue.

@dankamongmen
Copy link
Owner Author

looks like a solid fix. i think it's overeager about damage, however. right now i've taken paint_sixel() and on every cell we've got:

      }else if(!crender->p && !crender->s.bgblends){                                                       
        // if we are a bitmap, and above a cell that has changed (and                                      
        // will thus be printed), we'll need redraw the sprixel.                                           
        if(crender->sprixel == NULL){                                                                      
          crender->sprixel = s;                                                                            
        }                                                                                                  
        if(state == SPRIXCELL_ANNIHILATED || state == SPRIXCELL_ANNIHILATED_TRANS){                        
//fprintf(stderr, "REBUILDING AT %d/%d\n", y, x);                                                          
          sprite_rebuild(nc, s, y, x);                                                                     
        }                                                                                                  
      }

and i've added after sprite_rebuild():

//fprintf(stderr, "damaging due to rebuild [%s] %d/%d\n", nccell_extended_gcluster(crender->p, &crender->c)
          crender->s.damaged = 1;                                                                          

but really we only need damage if we changed and we rebuilt, right?

@dankamongmen
Copy link
Owner Author

so trying to put it at the logical place, in postpaint(), means it follows the sprite_rebuild(), and we're back out of ANNIHILATED state...yeah a better fix is to just check for SPRIXCELL_OPAQUE_SIXEL here:

  if(cellcmp_and_dupfar(pool, prevcell, crender->p, targc) > 0){                                           
//fprintf(stderr, "damaging due to cmp [%s] %d %d\n", nccell_extended_gcluster(crender->p, &crender->c), y,
    if(crender->sprixel){                                                                                  
      sprixcell_e state = sprixel_state(crender->sprixel, y, *x);                                          
fprintf(stderr, "state under candidate sprixel: %d %d/%d\n", state, y, *x);                                
      if(!crender->s.p_beats_sprixel && SPRIXCELL_OPAQUE_KITTY && state != SPRIXCELL_OPAQUE_SIXEL){        
fprintf(stderr, "damaged due to opaque %d/%d\n", y, *x);                                                   
        crender->s.damaged = 1;                                                                            
      }                                                                                                    
    }else{   

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bitmaps bitmapped graphics (sixel, kitty, mmap) bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant