Don't understand what's wrong with my PCX code

Question

I have a code that shows a PCX using assembly
By its width

Example image 320x200 x=0, y=0

But if X (StartPictX) and Y (StartPictY) are unequal to 0, it ruins the picture.

And I need to fix that...

The original code:

;--------------------------------------
; Load and show file *.PCX 320x200x256
;
;--------------------------------------
        IDEAL

        MODEL large
P386

MACRO SHOWPCX StartX, StartY, fName 
        mov ax, [StartX]
        mov [Point_X], ax
        mov ax, [StartY]
        mov [Point_Y], ax

        mov dx, offset fName

        call ShowPCXFile
ENDM SHOWPCX

    STACK 256

    DATASEG

    ErrorReadingFile DB 'Can not open file$'

        FileName1       DB 'mouse.pcx',0
        FileName        DW ?    ; offset file name for current file

        FileHandle      DW ?

        FileSize        DW ?

        ImageSizeInFile DW ?

        ImageWidth      DW ?
        ImageHeigth     DW ?

        PaletteOffset   DW ?

        Point_X         DW ?
        Point_Y         DW ?
        Color           DB ?

        StartPictX      DW ?
        StartPictY      DW ?

SEGMENT FILEBUF para public  'DATA'  
        DB 65200 DUP(?)
ENDS

        CODESEG   

Start:
        mov ax, @data
        mov ds, ax

        mov ax, 0013h
        int 10h
; -----------------= Show Pic1 =----------------
        mov [StartPictX], 0
        mov [StartPictY], 0

        SHOWPCX StartPictX, StartPictY, FileName1 

        mov ah,00h
        int 16h

        mov ah,00h
        mov al,03h
        int 10h
Exit:
        mov ax,04c00h
        int 21h

;-------------------------------------------
; ReadPCXFile - read PCX file into FILEBUF 
;-------------------------------------------
; Input:
;   File name
; Output:
;   File into FILEBUF
; Registers
;       AX, BX, CX, DX, DS
;-------------------------------------------
PROC ReadPCXFile Near
        pusha

;-----  Initialize variables
        mov     [FileHandle],0
        mov     [FileSize],0

;-----  Open file for reading
        mov     ah, 3Dh
        mov     al, 0
        ; mov DX,offset FileName  
        int     21h
        jc      @@Err
        mov     [FileHandle],AX   ; save Handle

;-----  Get the length of a file by setting a pointer to its end
        mov     ah, 42h
        mov     al ,2
        mov     bx, [FileHandle]
        xor     cx, cx
        xor     dx, dx
        int     21h
        jc      @@Err
        cmp     dx,0
        jne     @@Err  ;file size exceeds 64K

;-----  Save size of file
        mov     [FileSize], ax

;----- Return a pointer to the beginning of the file
        mov     ah, 42h
        mov     al, 0
        mov     bx, [FileHandle]
        xor     cx, cx
        xor dx, dx
        int 21h
        jc  @@Err

;-----  Read file into FILEBUF
        mov     bx, [FileHandle]
        pusha     
        push    ds
        mov     ax,FILEBUF
        mov     ds, ax
        xor     dx, dx
        mov     cx, 65200
        mov     ah, 3Fh
        int     21H
        pop     ds
        popa
        jc      @@Err

;-----  Close the file
        mov     ah, 3Eh
        mov     bx,[FileHandle]
        int     21H
        jc      @@Err
        popa
        ret

;-----  Exit - error reading file
@@Err:  ; Set text mode
        mov     ax, 3
        int     10h

        mov     dx, offset ErrorReadingFile
        mov     ah, 09h
        int     21h
        jmp     Exit
ENDP ReadPCXFile

;-------------------------------------------
; ShowPCXFile - show PCX file 
;-------------------------------------------
; Input:
;   File name
; Output:
;   The file
; Registers
;    AX, BX, CX, DX, DS
;-------------------------------------------
PROC ShowPCXFile Near   
        pusha

        call    ReadPCXFile

        mov     ax, FILEBUF
        mov     es, ax

;-----  Set ES:SI on the image
        mov     si, 128

;-----  Calculate the width and height of the image
        mov     ax, [es:42h]
        mov     [ImageWidth], ax
        dec     [ImageWidth]

        mov     ax, [es:0Ah]
        sub     ax, [es:6]
        inc     ax
        mov     [ImageHeigth], ax

;-----  Calculate the offset from the beginning of the palette file
        mov     ax, [FileSize]
        sub     ax, 768
        mov     [PaletteOffset], ax
        call    SetPalette
        mov     ax, [FileSize]
        sub     ax, 128+768
        mov     [ImageSizeInFile], ax

        xor  ch, ch            ; Clear high part of CX for string copies
        push [StartPictX]      ; Set start position
        pop  [Point_x]
        push [StartPictY]
        pop  [Point_y]
NextByte:
        mov     cl, [es:si]     ; Get next byte
        cmp     cl, 0C0h        ; Is it a length byte?
        jb      normal          ;  No, just copy it
        and     cl, 3Fh         ; Strip upper two bits from length byte
        inc     si              ; Advance to next byte - color byte

    mov     al, [es:si]
    mov     [Color], al
NextPixel:
        call    PutPixel
        cmp     cx, 1
        je  CheckEndOfLine

        inc     [Point_X]

        loop    NextPixel       
        jmp     CheckEndOfLine
Normal:
        mov     [Color], cl
        call    PutPixel

CheckEndOfLine:
        mov     ax, [Point_X]
        sub     ax, [StartPictX]
        cmp     ax, [ImageWidth]
;-----  [Point_X] - [StartPictX] >= [WidthPict] 
        jae     LineFeed
        inc     [Point_x]
        jmp     cont
LineFeed:
        push    [StartPictX]
        pop     [Point_x]
        inc     [Point_y]
cont:
        inc     si
        cmp     si, [ImageSizeInFile]     ; End of file? (written 320x200 bytes)
        jb      nextbyte
        popa
        ret
ENDP ShowPCXFile

;-------------------------------------------
; PutPixel - draw pixel 
;-------------------------------------------
; Input:
;   x - Point_x, y - Point_y, Color - color
; Output:
;   The pixel
; Registers
;    AX, BH, CX, DX
;-------------------------------------------
PROC PutPixel near
        pusha
        mov     bh, 0h
        mov     cx, [Point_x]
        mov     dx, [Point_Y]
        mov     al, [color]
        mov     ah, 0ch
        int     10h
        popa
        ret
ENDP PutPixel       

;-------------------------------------------
; SetPalette - change palette from 0-255 to from 0-63 
;-------------------------------------------
; Input:
;   PaletteOffset
; Output:
;   New palette
; Registers
;    AX, BX, CX, DX, SI, ES
;-------------------------------------------
SetPalette:
        pusha
        mov cx, 256*3
        mov si, [PaletteOffset]     
NextColor:
        shr [byte es:si], 2
        inc si
        loop NextColor

        mov dx, [PaletteOffset]
        mov ax, 1012h
        mov bx, 00h
        mov cx, 256d  
        int 10h
        popa
ret
        End Start

I realized the problem is the width of the image. It continues to line pixels until we reach the width of the image and then add to Y.
I tried to change it but didn't succeed:

CheckEndOfLine:
    mov     ax, [Point_X]
    ;sub     ax, [StartPictX]
    cmp     ax, 320
    jae     LineFeed
    inc     [Point_x]
    inc     si
    jmp     cont
LineFeed:
        push    [StartPictX]
        pop     [Point_x]
        inc     [Point_y]

        mov     bx,320
        sub     bx,[StartPictX]

@@lop:
        mov     cl, [es:si]     ; Get next byte
        cmp     cl, 0C0h        ; Is it a length byte?
        jb      nor
        and     cl, 3Fh         ; Strip upper two bits from length byte
        add     si,cx
        sub     bx,cx
        jmp next
nor:
        inc si
        dec bx
next:   
        cmp bx,1
        jg @@lop
cont:
        cmp     si, [ImageSizeInFile]     ; End of file? (written 320x200 bytes)
        jb      nextbyte
        popa
        ret

I really need help :)
Tnx for the helpers

What does `StartPictX` and `StartPictY` refer to? A position on the screen, or a position in the image? — Michael, Apr 24 '20 at 14:36
And while it may have nothing to do with your error, calling that `PutPixel` procedure for every pixel seems extremely inefficient. Why don't you just write directly to VGA memory? — Michael, Apr 24 '20 at 14:40
It doesn't seem like your `NextPixel` loop will handle if it reaches the end of a screen line. And where do you check if `Point_y` has reached 200? — Michael, Apr 24 '20 at 14:59
i didnt check if Point_y has reached 200 first i dont get why the x dont work and then i check y — Gad, Apr 24 '20 at 15:12

score 1 · Accepted Answer · answered Apr 26 '20 at 15:24

The calculation of the ImageSizeInFile is correct but your code uses this info as if it represented an address!. I suggest you rename the variable and use next calculation:

    mov     ax, [FileSize]
    sub     ax, 768
    mov     [ImageEndAddressInFile], ax

You need to calculate the ImageWidth correctly, just like you did for the ImageHeight. The number of bytes in 1 scanline does not necessarily correspond to the image width in pixels, especially in case of compression!

    mov     ax, [es:0008h]     ; X2
    sub     ax, [es:0004h]     ; X1
    inc     ax
    mov     [ImageWidth], ax

    mov     ax, [es:000Ah]     ; Y2
    sub     ax, [es:0006h]     ; Y1
    inc     ax
    mov     [ImageHeight], ax

Your current program increments the X position Point_x only conditionally. This is not fine. For every pixel that you've processed, you must always have inc [Point_x].

NextByte:
    mov     cl, [es:si]     ; Get next byte
    inc     si
    cmp     cl, 0C0h        ; Is it a length byte?
    jb      Normal          ;  No, just copy it

    and     cx, 003Fh       ; Strip upper two bits from length byte
    mov     al, [es:si]     ; The color
    inc     si
    mov     [Color], al
    jcxz    CheckEndOfLine  ; Could exist this one!
NextPixel:
    call    PutPixel
    inc     [Point_X]
    loop    NextPixel       
    jmp     CheckEndOfLine

Normal:
    mov     [Color], cl
    call    PutPixel
    inc     [Point_x]

CheckEndOfLine:
    mov     ax, [Point_X]
    sub     ax, [StartPictX]   ; Number of processed pixels
    cmp     ax, [ImageWidth]   ; Image width measured in pixels
    jb      Cont
LineFeed:
    push    [StartPictX]
    pop     [Point_x]
    inc     [Point_y]
Cont:
    cmp     si, [ImageEndAddressInFile]
    jb      NextByte

Now we can address the issue of freely selecting StartPictX and StartPictY.

An easy solution is to process the image data fully (just as if the image would snugly fit the screen) and have the PutPixel procedure clip off points that fall outside of the screen.

PROC PutPixel near
    cmp     [Point_y], 200
    jnb     IsOutside
    cmp     [Point_x], 320
    jnb     IsOutside
    pusha
    mov     dx, [Point_y]
    mov     cx, [Point_x]
    mov     bh, 0
    mov     al, [Color]
    mov     ah, 0Ch
    int     10h
    popa
  IsOutside:
    ret
ENDP PutPixel

A mixed solution is somewhat better. Always presenting PutPixel a valid Y-coordinate:

LineFeed:
    push    [StartPictX]
    pop     [Point_x]
    inc     [Point_y]
    cmp     [Point_y], 200          <<<<<
    jnb     Done                    <<<<<
Cont:
    cmp     si, [ImageEndAddressInFile]
    jb      NextByte
Done:

and

PROC PutPixel near
    cmp     [Point_x], 320
    jnb     IsOutside
    pusha
    mov     dx, [Point_y]
    mov     cx, [Point_x]
    mov     bh, 0
    mov     al, [Color]
    mov     ah, 0Ch
    int     10h
    popa
  IsOutside:
    ret
ENDP PutPixel

Don't understand what's wrong with my PCX code

1 Answers1

Now we can address the issue of freely selecting StartPictX and StartPictY.

Linked