7
\$\begingroup\$

Following the successful review of my implementation of Bresenham's Line Algorithm, I've been asked to upload the full implementation of my project which creates a small animation of shapes moving around the screen and changing colours for a full review.

Note that this code hasn't been edited to incorporate the comments made in the last review and as such I don't expect the line algorithm to be reviewed again. (Because of this it still lacks comments and so will still be difficult to review).

Here's a screenshot of the working program, and yes, it looks terrible when a screenshot is taken partway through the drawing.

enter image description here

;=========================================================================================================
; MACROS
;=========================================================================================================
;=========================================================================================================
; BRESENHAM LINE ALGORITHM (lx1,ly1)-(lx2,ly2)
;=========================================================================================================
line macro lx1, ly1, lx2, ly2
 local ldxsetup1, ldxsetup2, ldysetup1, ldysetup2
 local lxisetup1, lxisetup2, lxisetupexit, lyisetup1, lyisetup2, lyisetupexit
 local numsetup1, numsetup2, numsetupexit
 local lloopstart, lloopif, lloopifexit, lloopend
 pushall
 mov ax, lx2
 sub ax, lx1
 cmp ax, 0
 jge ldxsetup2
ldxsetup1:
 mov bx, -1
 mul bx
ldxsetup2:
 mov ldx, ax
 mov ax, ly2
 sub ax, ly1
 cmp ax, 0
 jge ldysetup2
ldysetup1:
 mov bx, -1
 mul bx
ldysetup2:
 mov ldy, ax
 mov ax, lx1
 mov lx, ax
 mov ax, ly1
 mov ly, ax
 mov ax, lx2
 cmp ax, lx1
 jge lxisetup1
 jmp lxisetup2
lxisetup1:
 mov ax, 1
 jmp lxisetupexit
lxisetup2:
 mov ax, -1
lxisetupexit:
 mov lxi1, ax
 mov lxi2, ax
 mov ax, ly2
 cmp ax, ly1
 jge lyisetup1
 jmp lyisetup2
lyisetup1:
 mov ax, 1
 jmp lyisetupexit
lyisetup2:
 mov ax, -1
lyisetupexit:
 mov lyi1, ax
 mov lyi2, ax
 mov ax, ldx
 mov bx, ldy
 cmp ax, bx
 jge numsetup1
 jmp numsetup2
numsetup1:
 mov ax, 0
 mov lxi1, ax
 mov lyi2, ax
 mov ax, ldx
 mov lden, ax
 mov lnumpix, ax
 shr ax, 1
 mov lnum, ax
 mov ax, ldy
 mov lnumadd, ax
 jmp numsetupexit
numsetup2:
 mov ax, 0
 mov lxi2, ax
 mov lyi1, ax
 mov ax, ldy
 mov lden, ax
 mov lnumpix, ax
 shr ax, 1
 mov lnum, ax
 mov ax, ldx
 mov lnumadd, ax
numsetupexit:
 mov ax, lnum
 mov dx, 0
lloopstart:
 cmp dx, lnumpix
 jg lloopend
 plot lx, ly
 add ax, lnumadd
 cmp ax, lden
 jge lloopif
 jmp lloopifexit
lloopif:
 sub ax, lden
 mov bx, lx
 add bx, lxi1
 mov lx, bx
 mov bx, ly
 add bx, lyi1
 mov ly, bx
lloopifexit:
 mov bx, lx
 add bx, lxi2
 mov lx, bx
 mov bx, ly
 add bx, lyi2
 mov ly, bx
 inc dx
 jmp lloopstart
lloopend:
 popall
endm
;=========================================================================================================
;=========================================================================================================
; DRAW TRIANGLE BOTTOM LEFT POINT AT (tx,ty) OF HEIGHT th AND WIDTH tw
;=========================================================================================================
triangle macro tx, ty, tw, th
 pushall
 mov ax, tx
 add ax, tw
 mov tx2,ax
 mov bx, tw
 shr bx, 1
 add bx, tx
 mov tx3, bx
 mov cx, ty
 sub cx, th
 mov ty3, cx
 line tx, ty, tx2, ty
 line tx2, ty, tx3, ty3
 line tx3, ty3, tx, ty
 popall
endm
;=========================================================================================================
;=========================================================================================================
; DRAW SHAPE
;=========================================================================================================
draw macro
 pushall
 mov ax, currentshape
 cmp currentshape, 1
 je draw1
 jmp draw2
draw1:
 mov ax, 0
 add ax, currentx
 mov val1, ax
 mov ax, 50
 add ax, currenty
 mov val2, ax
 mov ax, 50
 mov val3, ax
 mov ax, 43
 mov val4, ax
 triangle val1,val2,val3,val4
 mov ax, 25
 add ax, currentx
 mov val1, ax
 mov ax, 93
 add ax, currenty
 mov val2, ax
 triangle val1,val2,val3,val4
 mov ax, 50
 add ax, currentx
 mov val1, ax
 mov ax, 50
 add ax, currenty
 mov val2, ax
 triangle val1,val2,val3,val4
 mov ax, 25
 add ax, currentx
 mov val1, ax
 mov ax, 7
 add ax, currenty
 mov val2, ax
 mov ax, -43
 mov val4, ax
 triangle val1,val2,val3,val4
 mov ax, 0
 add ax, currentx
 mov val1, ax
 mov ax, 50
 add ax, currenty
 mov val2, ax
 mov ax, -43
 mov val4, ax
 triangle val1,val2,val3,val4
 mov ax, 50
 add ax, currentx
 mov val1, ax
 mov ax, 50
 add ax, currenty
 mov val2, ax
 mov ax, -43
 mov val4, ax
 triangle val1,val2,val3,val4
 jmp drawend
draw2:
 mov ax, 50
 add ax, currentx
 mov val1, ax
 mov ax, 0
 add ax, currenty
 mov val2, ax
 mov ax, 68
 add ax, currentx
 mov val3, ax
 mov ax, 3
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 79
 add ax, currentx
 mov val3, ax
 mov ax, 9
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 88
 add ax, currentx
 mov val3, ax
 mov ax, 18
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 94
 add ax, currentx
 mov val3, ax
 mov ax, 26
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 98
 add ax, currentx
 mov val3, ax
 mov ax, 38
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 100
 add ax, currentx
 mov val3, ax
 mov ax, 50
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 98
 add ax, currentx
 mov val3, ax
 mov ax, 62
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 94
 add ax, currentx
 mov val3, ax
 mov ax, 74
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 88
 add ax, currentx
 mov val3, ax
 mov ax, 82
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 79
 add ax, currentx
 mov val3, ax
 mov ax, 91
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 68
 add ax, currentx
 mov val3, ax
 mov ax, 97
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 50
 add ax, currentx
 mov val3, ax
 mov ax, 100
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 32
 add ax, currentx
 mov val3, ax
 mov ax, 97
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 21
 add ax, currentx
 mov val3, ax
 mov ax, 91
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 12
 add ax, currentx
 mov val3, ax
 mov ax, 82
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 6
 add ax, currentx
 mov val3, ax
 mov ax, 74
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 2
 add ax, currentx
 mov val3, ax
 mov ax, 62
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 0
 add ax, currentx
 mov val3, ax
 mov ax, 50
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 2
 add ax, currentx
 mov val3, ax
 mov ax, 38
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 6
 add ax, currentx
 mov val3, ax
 mov ax, 26
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 12
 add ax, currentx
 mov val3, ax
 mov ax, 18
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 21
 add ax, currentx
 mov val3, ax
 mov ax, 9
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 32
 add ax, currentx
 mov val3, ax
 mov ax, 3
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
 mov ax, val3
 mov val1, ax
 mov ax, val4
 mov val2, ax
 mov ax, 50
 add ax, currentx
 mov val3, ax
 mov ax, 0
 add ax, currenty
 mov val4, ax
 line val1,val2,val3,val4
drawend:
 popall
endm
;=========================================================================================================
;=========================================================================================================
; PLOT POINT (px,py)
;=========================================================================================================
plot macro px, py
 pushall
 mov ax, py
 mov bx, 320
 mul bx
 add ax, px
 mov di, ax
 mov al, colour
 mov es:[di],al
 popall
endm
;=========================================================================================================
;=========================================================================================================
; CLEAR SCREEN
;=========================================================================================================
cls macro
 pushall
 mov ah, 07h
 mov al, 00h
 mov bh, 00h
 mov cx, 00h
 mov dx, 1827h
 int 10h
 popall
endm
;=========================================================================================================
;=========================================================================================================
; PUSH ALL DATA TO STACK
;=========================================================================================================
pushall macro
 push ax
 push bx
 push cx
 push dx
endm
;=========================================================================================================
;=========================================================================================================
; POP ALL DATA FROM STACK
;=========================================================================================================
popall macro
 pop dx
 pop cx
 pop bx
 pop ax
endm
;=========================================================================================================
;=========================================================================================================
;=========================================================================================================
; SETUP
;=========================================================================================================
.model small
.stack 256
;=========================================================================================================
;=========================================================================================================
; VARIABLES
;=========================================================================================================
.data
;draw line variables
ldx dw 0
ldy dw 0
lx dw 0
ly dw 0
lxi1 dw 0
lxi2 dw 0
lyi1 dw 0
lyi2 dw 0
lden dw 0
lnum dw 0
lnumadd dw 0
lnumpix dw 0
;draw triangle variables
tx2 dw 0
tx3 dw 0
ty3 dw 0
;main program variables
startaddr dw 0a000h
colour db 4
currentx dw 0
currenty dw 0
currentshape dw 0
changeshape dw 0
val1 dw 0
val2 dw 0
val3 dw 0
val4 dw 0
dir dw 0
;=========================================================================================================
;=========================================================================================================
; PROGRAM START
;=========================================================================================================
.code
start:
 mov ax, @data
 mov ds, ax
 mov ah, 00h
 mov al, 13h
 int 10h
 mov es, startaddr
;=========================================================================================================
;=========================================================================================================
; MAIN PROGRAM
;=========================================================================================================
 mov ax, 10
 mov currentx, ax
 mov ax, 10
 mov currenty, ax
 mov ax, 2
 mov currentshape, ax
 mov ax, 0
 mov changeshape, ax
mainloop:
 mov ax, dir
 cmp ax, 1
 je dircheck1
 cmp ax, 2
 je dircheck2
 cmp ax,3
 je dircheck3
 jmp dircheck4
dircheck1:
 mov ax, 1
 add currentx, ax
 mov ax, currentx
 cmp ax, 210
 jle dircheckend
 mov ax, 2
 mov dir, ax
 mov al, colour
 inc al
 mov colour, al
 jmp dircheckend
dircheck2:
 mov ax, 1
 add currenty, ax
 mov ax, currenty
 cmp ax, 90
 jle dircheckend
 mov ax, 3
 mov dir, ax
 mov al, colour
 inc al
 mov colour, al
 mov ax, 1
 mov changeshape, ax
 jmp dircheckend
dircheck3:
 mov ax, -1
 add currentx, ax
 mov ax, currentx
 cmp ax, 10
 jge dircheckend
 mov ax, 4
 mov dir, ax
 mov al, colour
 inc al
 mov colour, al
 jmp dircheckend
dircheck4:
 mov ax, -1
 add currenty, ax
 mov ax, currenty
 cmp ax, 10
 jge dircheckend
 mov ax, 1
 mov dir, ax
 mov al, colour
 inc al
 mov colour, al
 mov ax, 1
 mov changeshape, ax
 jmp dircheckend
dircheckend:
 mov ax, changeshape
 cmp ax, 1
 jne shapechangeend
 mov ax, 0
 mov changeshape, ax
 mov ax, currentshape
 inc ax
 mov currentshape, ax
 cmp ax, 3
 jne shapechangeend
 mov ax, 1
 mov currentshape, ax
shapechangeend:
 cls
 draw
 mov ah, 01h
 int 16h
 jnz finish
 jmp mainloop
;========================================================================================================= 
;=========================================================================================================
; PROGRAM END
;=========================================================================================================
finish:
 mov ah, 00h
 mov al, 03h
 int 10h
 mov ah,04ch
 mov al,00h
 int 21h
end start
;=========================================================================================================
asked Mar 5, 2017 at 15:31
\$\endgroup\$
2
  • 1
    \$\begingroup\$ There seems to be a fair amount of duplicate code, have you tried reducing it? \$\endgroup\$ Commented Mar 5, 2017 at 17:01
  • \$\begingroup\$ @D.Jurcau no, this is an old project that I worked on and am interested in touching up following a full review. \$\endgroup\$ Commented Mar 5, 2017 at 17:36

1 Answer 1

5
\$\begingroup\$

Analysis (in random order)

mov ah, 00h
mov al, 03h
int 10h
mov ah,04ch
mov al,00h
int 21h

We always strive to get the smallest code without loosing readability. I think next snippet does a good job (it's 2 bytes smaller and well commented):

mov ax, 0003h ;AH=00h BIOS.SetVideo, AL=3 Textmode 80x25
int 10h
mov ax, 4C00h ;AH=4Ch DOS.Terminate, AL=0 ExitCode
int 21h

mov ax, 10
mov currentx, ax
mov ax, 10
mov currenty, ax
mov ax, 2
mov currentshape, ax
mov ax, 0
mov changeshape, ax

It's perfectly possible to move the numbers in the variables without putting them in a register first. But in the case where the same number goes into more than 1 variable using especially the AX register is advantageous as it reduces code size.

mov ax, 10
mov currentx, ax
mov currenty, ax
mov currentshape, 2
mov changeshape, 0

mov ax, 1
add currentx, ax
mov ax, currentx

The 3rd instruction here tells me that the value 1 that you moved into the AX register isn't all that important after the addition, so why not just increment the currentx variable?

inc currentx
mov ax, currentx

mov al, colour
inc al
mov colour, al

Again a perfect occasion to use less instructions and just increment the variable in one go:

inc colour

pushall
mov ax, py
mov bx, 320
mul bx
add ax, px
mov di, ax
mov al, colour
mov es:[di],al
popall

The plot macro code can be a bit smaller and use a register less if you wrote:

pushall
mov ax, 320
mul py <-- No need to use BX here
add ax, px
mov bx, ax
mov al, colour
mov es:[bx], al
popall

Note that I didn't use the DI register like you did. Because the pushall and popall macros don't actually preserve it, this could easily become a death-trap!
Although using macros to push/pop registers is easy, it also brings about pushing/popping too much, like in this code the CX register that isn't used at all.


pushall
mov ax, tx
add ax, tw 
mov tx2,ax
mov bx, tw
shr bx, 1
add bx, tx
mov tx3, bx
mov cx, ty
sub cx, th
mov ty3, cx
line tx, ty, tx2, ty
line tx2, ty, tx3, ty3
line tx3, ty3, tx, ty
popall

I'll use the triangle code to show many opportunities to shorten the code. mov bx, tw was moved to the top to avoid reading the tw variable twice.

push ax <-- Shorter than the pushall code
push bx
mov bx, tw
mov ax, tx
add ax, bx <-- Shorter between 2 registers 
mov tx2, ax
shr bx, 1
add bx, tx
mov tx3, bx
mov ax, ty <-- Shorter using AX instead of CX
sub ax, th
mov ty3, ax <-- Shorter using AX instead of CX
line tx, ty, tx2, ty
line tx2, ty, tx3, ty3
line tx3, ty3, tx, ty
pop bx
pop ax <-- Shorter than the popall code

Similar code to the next one is often repeated in the draw macro.

mov ax, 0
add ax, currentx
mov val1, ax
mov ax, 50
add ax, currenty
mov val2, ax
mov ax, 50
mov val3, ax
mov ax, 43
mov val4, ax

Why not simple mov the variable in the register?

mov ax, currentx
mov val1, ax

Changing the order gives shorter code:

mov ax, currenty
add ax, 50
mov val2, ax

Move the number in one go:

mov val3, 50
mov val4, 43

Conclusion

As you can see it's possible to optimize this code a lot, but I think the best advice I can offer is that all these macros really should have been written as subroutines. In this form the program is simply too long for what it does.

I tried to not repeat comments that I made on the previous (shorter) review. Obviously they still count.

I'm pretty sure that if all these modifications were applied correctly, and thus shortening the program considerably, more advanced tips (especially on program flow) could be given.

answered Mar 5, 2017 at 20:32
\$\endgroup\$
1
  • 1
    \$\begingroup\$ "...all these macros really should have been written as subroutines..." Beware that this cannot apply to the PUSHALL/POPALL macros! Either keep these or just write the required pushes/pops individually like @SepRoland did in the TRIANGLE code. \$\endgroup\$ Commented Mar 11, 2017 at 14:50

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.