fix: allow shadow flags in gCREATEBIT

even though they are technically incorrect for this API. Also errors from gCREATE, gCREATEBIT and draw operations now return an error to the program rather than silently continuing. Also allow SPRITEDELETE: with an invalid id. (Also needed to keep MBornes5 happy) fixes #444.
inseven · Oct 18, 2024 · 966643d · 966643d
1 parent 895e6b5
commit 966643d
Show file tree

Hide file tree

Showing 10 changed files with 97 additions and 39 deletions.
diff --git a/OpoLua/Extensions/CGContext.swift b/OpoLua/Extensions/CGContext.swift
@@ -28,7 +28,7 @@ extension CGContext {
         return CGAffineTransform(scaleX: 1.0, y: -1.0).translatedBy(x: 0.0, y: -CGFloat(self.height))
     }
 
-    func draw(_ operation: Graphics.DrawCommand, provider: DrawableImageProvider) {
+    func draw(_ operation: Graphics.DrawCommand, provider: DrawableImageProvider) -> Graphics.Error? {
         let col: CGColor
         if operation.mode == .clear {
             col = operation.bgcolor.cgColor()
@@ -76,7 +76,7 @@ extension CGContext {
         case .copy(let src, let mask):
             guard let srcImage = provider.getImageFor(drawable: src.drawableId) else {
                 print("Failed to get image for .copy operation!")
-                return
+                return .badDrawable
             }
 
             // Clip the rect to the source size to make sure we don't inadvertently stretch it
@@ -113,7 +113,7 @@ extension CGContext {
             }
             guard let srcImage = srcImage else {
                 print("Failed to get image for .pattern operation id=\(info.drawableId.value))!")
-                return
+                return .badDrawable
             }
             drawUnflippedImage(srcImage, in: info.rect.cgRect(), mode: operation.mode, tile: true)
         case .scroll(let dx, let dy, let rect):
@@ -206,7 +206,7 @@ extension CGContext {
 
                 if x <= operation.origin.x {
                     // There's nothing to underline (either text was empty or all spaces)
-                    return
+                    return nil
                 }
 
                 if fontInfo.flags.contains(.underlined) {
@@ -250,6 +250,7 @@ extension CGContext {
             self.restoreGState()
             */
         }
+        return nil
     }
 
     func draw(image: CGImage) {

diff --git a/OpoLua/Model/Program.swift b/OpoLua/Model/Program.swift
@@ -498,7 +498,7 @@ extension Program: OpoIoHandler {
         }
     }
 
-    func draw(operations: [Graphics.DrawCommand]) {
+    func draw(operations: [Graphics.DrawCommand]) -> Graphics.Error? {
         return DispatchQueue.main.sync {
             return windowServer.draw(operations: operations)
         }

diff --git a/OpoLua/Model/WindowServer.swift b/OpoLua/Model/WindowServer.swift
@@ -196,7 +196,7 @@ class WindowServer {
         if let cursorDrawCmd {
             if cursorCurrentlyDrawn {
                 // Hopefully this will un-draw it
-                window(for: cursorDrawCmd.drawableId)?.draw(cursorDrawCmd, provider: self)
+                let _ = window(for: cursorDrawCmd.drawableId)?.draw(cursorDrawCmd, provider: self)
                 cursorCurrentlyDrawn = false
             }
         }
@@ -207,7 +207,7 @@ class WindowServer {
             let col: Graphics.Color = cursor.flags.contains(.grey) ? .midGray : .black
             cursorDrawCmd = Graphics.DrawCommand(drawableId: cursor.id, type: op, mode: .invert, origin: cursor.rect.origin, color: col, bgcolor: .white, penWidth: 1, greyMode: .normal)
             print(cursorDrawCmd!)
-            window(for: cursor.id)?.draw(cursorDrawCmd!, provider: self)
+            let _ = window(for: cursor.id)?.draw(cursorDrawCmd!, provider: self)
             cursorCurrentlyDrawn = true
             if !cursor.flags.contains(.notFlashing) {
                 cursorTimer = Timer.scheduledTimer(withTimeInterval: kCursorFlashTime, repeats: true, block: { timer in
@@ -216,7 +216,7 @@ class WindowServer {
                         return
                     }
                     self.cursorCurrentlyDrawn = !self.cursorCurrentlyDrawn
-                    window.draw(cmd, provider: self)
+                    let _ = window.draw(cmd, provider: self)
                 })
             }
         }
@@ -313,14 +313,17 @@ class WindowServer {
         window.setSprite(canvasSprite, for: id)
     }
 
-    func draw(operations: [Graphics.DrawCommand]) {
+    func draw(operations: [Graphics.DrawCommand]) -> Graphics.Error? {
         for op in operations {
             guard let drawable = self.drawable(for: op.drawableId) else {
                 print("No drawable for drawableId \(op.drawableId)!")
-                continue
+                return .badDrawable
+            }
+            if let err = drawable.draw(op, provider: self) {
+                return err
             }
-            drawable.draw(op, provider: self)
         }
+        return nil
     }
 
     private func bringInfoWindowToFront() {

diff --git a/OpoLua/Views/Canvas.swift b/OpoLua/Views/Canvas.swift
@@ -33,7 +33,7 @@ protocol Drawable: AnyObject {
     var id: Graphics.DrawableId { get }
     var mode: Graphics.Bitmap.Mode { get }
 
-    func draw(_ operation: Graphics.DrawCommand, provider: DrawableImageProvider)
+    func draw(_ operation: Graphics.DrawCommand, provider: DrawableImageProvider) -> Graphics.Error?
     func getImage() -> CGImage?
 
 }
@@ -83,15 +83,18 @@ class Canvas: Drawable {
         context.fill(CGRect(x: 0, y: 0, width: context.width, height: context.height))
     }
 
-    func draw(_ operation: Graphics.DrawCommand, provider: DrawableImageProvider) {
+    func draw(_ operation: Graphics.DrawCommand, provider: DrawableImageProvider) -> Graphics.Error? {
+        defer {
+            self.image = nil
+        }
+
         if data != nil && operation.mode == .invert {
-            drawInverted(operation: operation, provider: provider)
+            return drawInverted(operation: operation, provider: provider)
         } else if data != nil, case .invert(_) = operation.type {
-            drawInverted(operation: operation, provider: provider)
+            return drawInverted(operation: operation, provider: provider)
         } else {
-            context.draw(operation, provider: provider)
+            return context.draw(operation, provider: provider)
         }
-        self.image = nil
     }
 
     func draw(image: CGImage) {
@@ -108,7 +111,7 @@ class Canvas: Drawable {
     }
 
     // Only supported when using 8bpp greyscale backing data
-    func drawInverted(operation: Graphics.DrawCommand, provider: DrawableImageProvider) {
+    func drawInverted(operation: Graphics.DrawCommand, provider: DrawableImageProvider) -> Graphics.Error? {
         let byteVal = ~operation.color.greyValue
         switch operation.type {
         case .fill(let size):
@@ -130,7 +133,7 @@ class Canvas: Drawable {
         case .copy(let src, _): // Mask is never used in gCOPY, only in gBUTTON impl which doesn't use invert
             guard let srcImg = provider.getImageFor(drawable: src.drawableId) else {
                 print("Failed to get image for .copy operation!")
-                return
+                return .badDrawable
             }
             // Hopefully don't have to deal with downscaling a colour bitmap into a greyscale canvas...
             assert(srcImg.bitsPerPixel == 8)
@@ -168,8 +171,9 @@ class Canvas: Drawable {
             drawLineInverted(x0: topLeft.x, y0: topLeft.y + size.height, x1: topLeft.x, y1: topLeft.y, value: byteVal) // bottom
         default:
             print("TODO: drawInverted \(operation.type)")
-            context.draw(operation, provider: provider)
+            return context.draw(operation, provider: provider)
         }
+        return nil
     }
 
     func getImage() -> CGImage? {

diff --git a/OpoLua/Views/CanvasView.swift b/OpoLua/Views/CanvasView.swift
@@ -61,20 +61,27 @@ class CanvasView : UIView, Drawable {
         }
     }
 
-    func draw(_ operation: Graphics.DrawCommand, provider: DrawableImageProvider) {
+    func draw(_ operation: Graphics.DrawCommand, provider: DrawableImageProvider) -> Graphics.Error? {
+        defer {
+            self.image = nil
+            setNeedsDisplay()
+        }
         if operation.greyMode.drawGreyPlane {
             precondition(self.mode == .gray4, "Bad window mode for a grey plane operation!")
             if self.greyPlane == nil {
-                // The grey plan canvas's mode doesn't really matter here so long as it translates to 8bpp greyscale
+                // The grey plane canvas's mode doesn't really matter here so long as it translates to 8bpp greyscale
                 self.greyPlane = Canvas(id: self.canvas.id, size: self.canvas.size, mode: .gray2)
             }
-            self.greyPlane?.draw(operation, provider: provider)
+            if let err = self.greyPlane?.draw(operation, provider: provider) {
+                return err
+            }
         }
         if operation.greyMode.drawNormalPlane {
-            canvas.draw(operation, provider: provider)
+            if let err = canvas.draw(operation, provider: provider) {
+                return err
+            }
         }
-        self.image = nil
-        setNeedsDisplay()
+        return nil
     }
 
     func setSprite(_ sprite: CanvasSprite?, for id: Int) {

diff --git a/src/opl.lua b/src/opl.lua
@@ -485,7 +485,11 @@ function gCREATE(x, y, w, h, visible, flags)
     -- printf("gCREATE w=%d h=%d flags=%X", w, h, flags or 0)
     local ctx = runtime:newGraphicsContext(w, h, true, (flags or 0) & 0xF)
     local id = ctx.id
-    runtime:iohandler().createWindow(id, x, y, w, h, flags or KColorgCreate2GrayMode)
+    local err = runtime:iohandler().createWindow(id, x, y, w, h, flags or KColorgCreate2GrayMode) or KErrNone
+    if err ~= KErrNone then
+        runtime:closeGraphicsContext(id)
+        error(err)
+    end
     -- printf(" id=%d\n", id)
     ctx.winX = x
     ctx.winY = y
@@ -497,9 +501,15 @@ end
 
 function gCREATEBIT(w, h, mode)
     -- printf("gCREATEBIT w=%d h=%d mode=%X", w, h, mode or 0)
+    -- Mask mode here because some apps incorrectly include shadow flags as per gCREATE
+    mode = (mode or 0) & 0xF
     local ctx = runtime:newGraphicsContext(w, h, false, mode)
     local id = ctx.id
-    runtime:iohandler().createBitmap(id, w, h, mode)
+    local err = runtime:iohandler().createBitmap(id, w, h, mode) or KErrNone
+    if err ~= KErrNone then
+        runtime:closeGraphicsContext(id)
+        error(err)
+    end
     -- printf(" id=%d\n", id)
     return id
 end

diff --git a/src/opx/bmp.lua b/src/opx/bmp.lua
@@ -93,6 +93,7 @@ end
 
 function SPRITECREATE(runtime, winId, x, y, flags)
     local graphics = runtime:getGraphics()
+    -- printf("SPRITECREATE(winId=%d, x=%d, y=%d, flags=%X", winId, x, y, flags)
     assert(graphics[winId] and graphics[winId].isWindow, "id is not a window")
     local spriteId = #graphics.sprites + 1
     local sprite = {
@@ -103,6 +104,7 @@ function SPRITECREATE(runtime, winId, x, y, flags)
     }
     graphics.sprites[spriteId] = sprite
     graphics.currentSprite = sprite
+    -- printf(" = %d\n", spriteId)
     return spriteId
 end
 
@@ -211,10 +213,17 @@ function SPRITEPOS(runtime, spriteId, x, y)
 end
 
 function SpriteDelete(stack, runtime)
-    -- printf("SpriteDelete\n")
+    local id = stack:pop()
+    -- printf("SpriteDelete %d\n", id)
     local graphics = runtime:getGraphics()
-    local sprite = graphics.sprites[stack:pop()]
-    assert(sprite, "Bad sprite ID!")
+    local sprite = graphics.sprites[id]
+
+    if sprite == nil then
+        -- It seems like this isn't an error on the Psion 5?
+        printf("Bad sprite ID %d in SpriteDelete!\n", id)
+        stack:push(0)
+        return
+    end
     for _, frame in ipairs(sprite.frames) do
         decRefcount(runtime, frame.bitmap)
         decRefcount(runtime, frame.mask)

diff --git a/src/runtime.lua b/src/runtime.lua
@@ -578,15 +578,21 @@ function Runtime:drawCmd(type, op)
     if graphics.buffer then
         table.insert(graphics.buffer, op)
     else
-        self.ioh.draw({ op })
+        local err = self.ioh.draw({ op }) or KErrNone
+        if err ~= KErrNone then
+            error(err)
+        end
     end
 end
 
 function Runtime:flushGraphicsOps()
     local graphics = self.graphics
     if graphics and graphics.buffer and graphics.buffer[1] then
-        self.ioh.draw(graphics.buffer)
+        local err = self.ioh.draw(graphics.buffer) or KErrNone
         graphics.buffer = {}
+        if err ~= KErrNone then
+            error(err)
+        end
     end
 end
 

diff --git a/swift/OpoInterpreter.swift b/swift/OpoInterpreter.swift
@@ -191,8 +191,12 @@ private func draw(_ L: LuaState!) -> CInt {
         ops.append(Graphics.DrawCommand(drawableId: id, type: optype, mode: mode, origin: origin, color: color,
                                         bgcolor: bgcolor, penWidth: penWidth, greyMode: greyMode))
     }
-    iohandler.draw(operations: ops)
-    return 0
+    if let err = iohandler.draw(operations: ops) {
+        L.push(err.rawValue)
+        return 1
+    } else {
+        return 0
+    }
 }
 
 func doGraphicsOp(_ L: LuaState!, _ iohandler: OpoIoHandler, _ op: Graphics.Operation) -> CInt {
@@ -209,6 +213,9 @@ func doGraphicsOp(_ L: LuaState!, _ iohandler: OpoIoHandler, _ op: Graphics.Oper
     case .peekedData(let data):
         L.push(data)
         return 1
+    case .error(let error):
+        L.push(error.rawValue)
+        return 1
     }
 }
 
@@ -474,7 +481,8 @@ private func createBitmap(_ L: LuaState!) -> CInt {
           let modeVal = L.toint(4),
           let mode = Graphics.Bitmap.Mode(rawValue: modeVal) else {
         print("Bad parameters to createBitmap")
-        return 0
+        L.push(Graphics.Error.invalidArguments.rawValue)
+        return 1
     }
     let drawableId = Graphics.DrawableId(value: id)
     let size = Graphics.Size(width: width, height: height)
@@ -488,7 +496,9 @@ private func createWindow(_ L: LuaState!) -> CInt {
           let width = L.toint(4), let height = L.toint(5),
           let flags = L.toint(6),
           let mode = Graphics.Bitmap.Mode(rawValue: flags & 0xF) else {
-        return 0
+        print("Bad parameters to createWindow")
+        L.push(Graphics.Error.invalidArguments.rawValue)
+        return 1
     }
 
     var shadow = 0

diff --git a/swift/OpoIoHandler.swift b/swift/OpoIoHandler.swift
@@ -374,6 +374,13 @@ public struct Graphics {
         case nothing
         case textMetrics(TextMetrics)
         case peekedData(Data)
+        case error(Error)
+    }
+
+    public enum Error: Int {
+        case invalidArguments = -2
+        case badDrawable = -118
+        case invalidWindow = -119
     }
 }
 
@@ -594,7 +601,7 @@ public protocol OpoIoHandler: FileSystemIoHandler {
 
     func beep(frequency: Double, duration: Double) -> Error?
 
-    func draw(operations: [Graphics.DrawCommand])
+    func draw(operations: [Graphics.DrawCommand]) -> Graphics.Error?
     func graphicsop(_ operation: Graphics.Operation) -> Graphics.Result
 
     func getDeviceInfo() -> (Graphics.Size, Graphics.Bitmap.Mode, String)
@@ -643,7 +650,8 @@ class DummyIoHandler : OpoIoHandler {
         return nil
     }
 
-    func draw(operations: [Graphics.DrawCommand]) {
+    func draw(operations: [Graphics.DrawCommand]) -> Graphics.Error? {
+        return nil
     }
 
     func graphicsop(_ operation: Graphics.Operation) -> Graphics.Result {