Making a screenshot using assembly

Question

I am working on a project in assembly. I want to add an option to make a screenshot. I am saving the picture in BMP format 256 colors bitmap. I am working in 320*200 graphic mode (in 16 bit assembly in DOSBox).
I have been working on this for a long time and could not find the problem in my code.
What I am doing is simply creating a file (without problems), adding the header (I am working on BMP format), copy the palette to the file (I am switching the red and green because BMP is written in BGR and not RGB) and then copy the pixels directly from the video memory (I am doing it upside down because BMP file is written in upside down).

I have been trying for a long time to solve it but I can't find my mistake. If someone can find my mistake please let me know.
Thank you very much.

    proc MakeScreenshot
    pusha
    mov bp, sp
    sub sp, 2

    @MakeScreenshot@lineCounter         equ [word ptr bp - 2]
    ; creating the file
        ; finding the last var in the arr
            push offset ScreenShotPath
            push 1000
            push 0
            call find

            cmp [index], -1 
            jne @MakeScreenshot@cont6
            call PError

    @MakeScreenshot@cont6:
        mov bl, [index]
        xor bh, bh
        add bx, offset ScreenShotPath
        sub bx, 6
        mov di, bx

    ; get the time
        ; CH = hour (0-23)
        ; CL = minutes (0-59)
        ; DH = seconds (0-59)
        mov ah, 2ch
        int 21h
        cmp dx, [SecAndMin]
        jne @MakeScreenshot@cont
            inc [byte ptr di + 1]
            jmp @MakeScreenshot@creat

    @MakeScreenshot@cont:
        mov [SecAndMin], dx
        mov dl, dh
        xor dh, dh
        mov ax, dx
        cmp ax, 10
        jae @MakeScreenshot@Div4
            mov [byte ptr di], '0'
            inc di
            add al, '0'
            mov [byte ptr di], al
            sub di, 4
            jmp @MakeScreenshot@file

    @MakeScreenshot@Div4:
    ; Divide the number to digits in the stack
        xor si, si                  ; Si counts the number of digits
        mov bl, 10
    @MakeScreenshot@dig5:
        div bl                      ; Divide ax by 10
        mov cl, ah
        xor ch, ch
        push cx                     ; Save the digit in the stack
        xor ah, ah
        inc si
        cmp ax, 0
        jne @MakeScreenshot@dig5

        mov cx, si
        mov si, di
    @MakeScreenshot@Sec:
        pop ax
        xor ah, ah
        add al, '0'
        mov [si], al
        inc si
        loop @MakeScreenshot@Sec

        sub di, 3

    @MakeScreenshot@file:
        ; get the time
        ; CH = hour (0-23)
        ; CL = minutes (0-59)
        ; DH = seconds (0-59)
        mov ah, 2ch
        int 21h
        xor ch, ch
        mov ax, cx
        cmp ax, 10
        jae @MakeScreenshot@Div5
            inc di
            mov [byte ptr di], '0'
            inc di
            add al, '0'
            mov [byte ptr di], al
            inc di
            jmp @MakeScreenshot@creat

    @MakeScreenshot@Div5:
    ; Divide the number to digits in the stack
        xor si, si                  ; Si counts the number of digits
        mov bl, 10
    @MakeScreenshot@dig6:
        div bl                      ; Divide ax by 10
        mov cl, ah
        xor ch, ch
        push cx                     ; Save the digit in the stack
        xor ah, ah
        inc si
        cmp ax, 0
        jne @MakeScreenshot@dig6

        mov cx, si
        inc di
        mov si, di
    @MakeScreenshot@Min:
        pop ax
        xor ah, ah
        add al, '0'
        mov [byte ptr si], al
        inc si
        loop @MakeScreenshot@Min


@MakeScreenshot@creat:
    push offset ScreenShotPath
    push offset ScreenShotHandle
    call CreateFile

; write the header
    mov ah, 40h
    mov bx, [ScreenShotHandle]  ; file handle
    mov cx, 54                  ; number of bytes to write
    mov dx, offset Fileheader   ; pointer to write buffer (the header is the same and we take a picture omly after the main page so i will not be empty)
    int 21h 
    jnc @MakeScreenshot@Cont20
        mov ah, 59h
        mov bx, 0
        int 21h
        push ax
        call PrintFileError
    @MakeScreenshot@Cont20:

; write the pallete
    push offset ScreenShotPalette
    call SavePalette
    mov cx, 256
    mov bx, offset ScreenShotPalette
    xor di, di
    @MakeScreenshot@copyPal:
        ; Note: The palette of BMP files are BGR and not RGB so it has to be upsidedown.

        ; Copy last/first color (blue)
            mov al, [bx + 2]    ; Get the last color of the palette to the first color (blue).
            shl al, 2           ; The BMP palette is larger. ThereFore multyplayer by 4 in order ro corp it to the right size.
            mov [byte ptr ScreenShotPalette2 + di], al
            inc di

        ; Do the same on the second color (green)
            mov al, [bx + 1] 
            shl al, 2
            mov [byte ptr ScreenShotPalette2 + di], al
            inc di

        ; Do the same on the first/last color (red)
            mov al, [bx]
            shl al, 2
            mov [byte ptr ScreenShotPalette2 + di], al
            inc di

        ; enter an empty byte
            mov [byte ptr ScreenShotPalette2 + di], 0
            inc di

        add bx, 4               ; Jump to the next color (each color is a double word (4 bytes))
        loop @MakeScreenshot@copyPal            ; It has to be 256 time becuase there are 256 colors im the palette.    

    mov ah, 40h
    mov bx, [ScreenShotHandle]          ; file handle
    mov cx, 256                         ; number of bytes to write
    mov dx, offset ScreenShotPalette2   ; pointer to write buffer 
    int 21h 
    jnc @MakeScreenshot@Cont2
        mov ah, 59h
        mov bx, 0
        int 21h
        push ax
        call PrintFileError

    @MakeScreenshot@Cont2:
    ; Copy the image
        ; Declare the address of the video memory
            mov ax, 0A000h          ; The video memory is stored on A000:0000
            mov es, ax

        ; Copy
            mov cx, 200             ; 200 lines 
            mov @MakeScreenshot@lineCounter, 0      ; Reset the line counter
        @MakeScreenshot@copyImage:
            push cx                 ; Saves the cx for the big loop

            ; Copy one line
                ; Declare where to copy from
                    mov di, offset ScrenshotLine

                ; Declare where start to write
                    mov si, @MakeScreenshot@lineCounter
                    mov ax, 320     ; We start to write a line on A000:lineNumber*320
                    mul si          ; Saves on dx:ax
                    mov si, ax      ; The max outcome fits into one word
                    ; Note: si now point where to write if the pic was not upside down

                    ; Make it 
                    mov ax, 63679   ; Ax = ((320 * 200) - 1) - 320 which is the start of the last line of the video memory 
                    sub ax, si      ; Ax now poit to the start of the oppesite line 
                    mov si, ax

                mov cx, 320
            @MakeScreenshot@copyLine:
                ; Copy from the stored line into the video memory
                    mov al, [es:si]
                    mov [ds:di], al ; Copy one byte from the stored line to the vide memory
                    inc si
                    inc di
                loop @MakeScreenshot@copyLine

            ; copy the line to the file
                mov ah, 40h
                mov bx, [ScreenShotHandle]          ; file handle
                mov cx, 320                         ; number of bytes to write
                mov dx, offset ScreenShotPalette2   ; pointer to write buffer 
                int 21h 
                jnc @AddToLog@Cont3
                    mov ah, 59h
                    mov bx, 0
                    int 21h
                    push ax
                    call PrintFileError
                @AddToLog@Cont3:
            inc @MakeScreenshot@lineCounter
            pop cx                  ; Return the value to cx
            loop @MakeScreenshot@copyImage

    ; close the file
        push [ScreenShotHandle]
        call closeFile

    add sp, 2                       ; Delete local var

    popa
    ret
endp MakeScreenshot

Answer 1

push offset ScreenShotPalette
call SavePalette
mov cx, 256
mov bx, offset ScreenShotPalette
xor di, di
@MakeScreenshot@copyPal:

The code for SavePalette isn't shown, but if it uses the video BIOS function ReadBlockOfColorRegisters (AX=1017h), then the buffer at ScreenShotPalette will hold 768 bytes (256 x 3). If so, your add bx, 4 should become add bx, 3.

mov ah, 40h
mov bx, [ScreenShotHandle]          ; file handle
mov cx, 256                         ; number of bytes to write
mov dx, offset ScreenShotPalette2   ; pointer to write buffer 
int 21h 

The palette is 1024 bytes (256 x 4). You write only 256!

---

Your copyImage code has a lot of misleading comments! I'm guessing that this code was first written to write an image to the screen and not to read it from the screen. Anyway, you store the pixels at ScrenshotLine (mov di, offset ScrenshotLine) but for writing these in your BMP file, you use a completely different address ScreenShotPalette2 (mov dx, offset ScreenShotPalette2).

The calculation for the address on screen is wrong. 63679 is not the address of the last line on the screen. That would be 63680.
Better news is that you don't need to use that local variable @MakeScreenshot@lineCounter. You can use the CX counter for the same purpose and avoid that difficult picture reversal operation.

    ; Copy
    mov cx, 200     ; 200 lines 
@MakeScreenshot@copyImage:
    push cx
    dec  cx         ; Current line 199, 198, 197, ...
    ; Copy one line
    mov  di, offset ScrenshotLine
    mov  ax, 320    \
    mul  cx         | These 3 together in 1 using "imul si, cx, 320"
    mov  si, ax     /
    mov  cx, 320
@MakeScreenshot@copyLine:
    mov  al, [es:si]
    mov  [ds:di], al
    inc  si
    inc  di
    loop @MakeScreenshot@copyLine
    ; copy the line to the file
    mov  ah, 40h
    mov  bx, [ScreenShotHandle]
    mov  cx, 320
    mov  dx, offset ScrenshotLine
    int  21h 
    ...
    pop  cx
    loop @MakeScreenshot@copyImage

---

Concerning the date from DOS.function 2Ch. You should NOT retrieve it twice. Use the info from the one call.