assemblyx86-16vga

Why an x86 assembly program paints the wrong symbol


I'm a university student and I can't figure out where my mistake is. Here's my assignment to write in x86 assembly language.(I wrote on an INTEL syntax, I use DosBOX to run it)

Fill page 0 of the video memory with any text. In the first line of the screen, display 16 NUL characters (ASCII code 0) with different values of the background attribute (the highest tetrad of the symbol attribute). Press the left button in the first line to select and store the selected color. When the cursor is placed on any character of the remaining lines of the screen, the color of the character will change to the selected one by pressing the left button.

BUT The program changes the color of a symbol other than the one I clicked on with my cursor.

.386
Data segment use16
    ColorOne db 2fh
    ColorMain db 0fh
    sym db 100
    style db ?
Data ends

Code segment use16
assume cs:Code, ds:Data
start:
    mov ax, DATA
    mov ds, ax
    mov ax, 0B800h
    mov es, ax

    mov ax, 3           ; 80x35
    int 10h 

    mov bx, 0
    xor di, di

    ; screen fill
loop1:
    add di, bx
    imul di, 2
    mov ah, ColorOne        
    mov al, 0

    stosw

    add ColorOne, 10h
    inc bx
    xor di, di
    cmp bx, 16
    jl loop1

    mov bx, 1

loop2:
    mov cx, 0
    loop3:
        add di, bx
        imul di, 80
        add di, cx
        imul di, 2
        mov ah, ColorMain       
        mov al, sym

        stosw

        inc cx
        xor di, di
        cmp cx, 80
        jl loop3
        
    inc bx
    inc sym
    cmp bx, 25
    jl loop2
;---------------------------------------------------------
    mov ax, 1           ; show cursor
    int 33h

    mov ax, 0ch
    mov cx, 11b         ; move and press left button

    push es     ; save segment status
    push cs
    pop  es

    lea dx, prmaus      ; set the offset of the event processing procedure from the mouse in the code segment

    int 33h         ; registration of the address and conditions of the call
    pop es

    mov ah, 01h     ; wait
    int 21h     ; pause

    xor cx,cx           ; disconect mouse
    mov ax,0ch      
    int 33h

    mov ax, 3           ; clean screen
    int 10h

    mov ax, 4c00h   ; exit
    int 21h 
;----------------------------------------------------------
prmaus proc far
    ; saving the contents of registers ds, es
    push ds
    push es
    pusha
    push 0b800h
    pop es  
    push Data
    pop ds

    ; algorithm
to_left_mouse:
    cmp bx, 1b
    jne to_end

    mov ax, 2               ; hide cursor
    int 33h

    first_row:
        cmp dx, 0           ; If DX is equal to 0, the mouse is over the first row
        jne another_row     ; If the mouse is not over the first row, jump to processing other rows
        shr cx, 2           ; Shift CX right by 2 bits (divide by 4, because each character takes up 4 bytes)
        mov di, cx          ; Store the value of CX in DI (for array indexing)
        mov bx, es:[di]     ; Get the value of the character color in the row
         shr bx, 8          ; Shift the color value right by 8 bits
        mov bh, bl          ; Copy the lower byte to the upper byte of the BH register (to preserve the color)
         mov style, bh      ; Store the color in the style variable

        jmp to_end          ; Jump to the end of the procedure

another_row:                ; Label for processing other rows of the window
        xor di, di          ; Zero out the index of the screen buffer

        add di, dx          ; Add the value of DX to DI (to get the row number)
        imul di, 20         ; Multiply the value of DI by 20 (the width of the window in characters)
        shr cx, 2           ; Shift CX right by 2 bits (divide by 4, because each character takes up 4 bytes)
        add di, cx          ; Add the value of CX to DI (to get the column number)

        mov ax, es:[di]     ; Get the value of the character from the screen buffer
        mov al, ah          ; Copy the upper byte to the lower byte (to preserve the character)

        mov ah, style       ; Store the color value in the AH register

        stosw               ; Save the value in the screen buffer

to_end:

    mov ax, 1           ; show cursor
    int 33h

    popa
    pop es
    pop ds
    retf
prmaus  endp
Code ends
end start

I suppose that maybe the problem is a two byte shift (but on the other hand I tried different variants and could not get the right result)

shr cx, 2 ; Shift CX right by 2 bits (divide by 4, because each character takes up 4 bytes)

Is expected that when you click on the cursor on the color will be selected , and after clicking on the symbol , it will change to the selected color


Solution

  • shr cx, 2           ; Shift CX right by 2 bits (divide by 4, because each character takes up 4 bytes)
    

    The reasoning in the comment is wrong! Shifting twice is only shortcut-arithmatic.
    Because the mouse driver gives you CX = 8 * Column, we need a division by 8, and because for every charactercell in the video memory we use a word, we need to multiply by 2. The combo means we need to divide by 4 which a double shift to the right can do most efficiently.

    imul di, 20         ; Multiply the value of DI by 20 (the width of the window in characters)
    

    The reasoning in the comment is wrong! Multiplying by 20 is only shortcut-arithmatic.
    Because the mouse driver gives you DX = 8 * Row, we need a division by 8, and because for every row of characters in the video memory we use 160 bytes, we need to multiply by 160. The combo means we need to multiply by 20.

    mov bh, bl          ; Copy the lower byte to the upper byte of the BH register (to preserve the color)
    mov al, ah          ; Copy the upper byte to the lower byte (to preserve the character)
    

    These operation aren't preserving anything! And in the latter case, it is the root problem of why "the program changes the color of a symbol other than the one I clicked on with my cursor".


    These are the fixes

        shr  cx, 2
        imul di, dx, 20
        add  di, cx
        test dx, dx
        jnz  another_row
    first_row:
        mov  al, es:[di+1]  ; Get the value of the attribute
        mov  style, al      ; Store the color in the style variable
        jmp  to_end         ; Jump to the end of the procedure
    another_row:
        mov  al, style
        mov  es:[di+1], al  ; Change the value of the attribute
    to_end:
    

    See that with es:[di+1] you can address the attribute byte directly.
    And imul di, dx, 20 (one of the finest x86 instructions around) does DI = DX * 20.