From d784dd3a52f1182d7d22c7308ce3469e9dd763fc Mon Sep 17 00:00:00 2001 From: Alberto Torres Date: Sat, 31 Aug 2024 00:40:02 +0200 Subject: [PATCH] Replace the use of pointer/len and "done()" by SliceMem and destructors. --- libs/loadable/loadable.nim | 123 ++++++++++------------------- src/gpu_formats/texture_decode.nim | 29 ++++--- src/graphics/texture.nim | 34 ++++---- src/loaders/blend.nim | 16 ++-- src/loaders/blend_format.nim | 7 -- 5 files changed, 81 insertions(+), 128 deletions(-) diff --git a/libs/loadable/loadable.nim b/libs/loadable/loadable.nim index f2d2e3c..46c51a0 100644 --- a/libs/loadable/loadable.nim +++ b/libs/loadable/loadable.nim @@ -42,6 +42,9 @@ ## ## TODO: investigate the use of signals instead of callbacks +{.hint[ConvFromXtoItselfNotNeeded]:off.} + +import arr_ref # for SliceMem type LoadableResourceStatus* = enum NotStarted @@ -49,27 +52,24 @@ type LoadableResourceStatus* = enum Finished Error -type Result = tuple[ok: bool, err: string, p: pointer, len: int] +# type Result = tuple[ok: bool, err: string, data: SliceMem[byte]] type LoadableResource* = ref object of RootObj status: LoadableResourceStatus - start_func: proc(self: LoadableResource) - onload_func: proc(ok: bool, err: string, p: pointer, len: int) - done_func: proc() - cancel_func: proc() + start_func: proc(self: LoadableResource) {.closure.} + onload_func: proc(ok: bool, err: string, data: SliceMem[byte]) {.closure.} + cancel_func: proc() {.closure.} str*: proc(): string use_threads: bool - result: ref Result # only to be used with loadAll + # result: ref Result # only to be used with loadAll proc newLoadableResource*[T: LoadableResource]( start: proc(self: LoadableResource), - done: proc() = nil, str: proc(): string = nil, use_threads = false, ): T = new(result) result.start_func = start - result.done_func = done result.str = str if str == nil: result.str = proc(): string = "" @@ -78,8 +78,7 @@ proc newLoadableResource*[T: LoadableResource]( # main -> thread channels var to_start: Channel[LoadableResource] # main <- thread channels -var to_return: Channel[(LoadableResource, bool, string, pointer, int)] -var to_done: Channel[LoadableResource] +var to_return: Channel[(LoadableResource, bool, string, SliceMem[byte])] proc start*[T: LoadableResource](self: T) = self.status = Started @@ -88,46 +87,24 @@ proc start*[T: LoadableResource](self: T) = else: self.start_func(self) -proc doneImpl*[T: LoadableResource](self: T) = - if self.status == Started: - self.cancel() - else: - self.status = NotStarted - if self.done_func != nil: - self.done_func() - -proc `onload=`*[T: LoadableResource](self: T, onload_func: proc(ok: bool, err: string, p: pointer, len: int)) = +proc `onload=`*[T: LoadableResource](self: T, onload_func: proc(ok: bool, err: string, data: SliceMem[byte])) = self.onload_func = onload_func -proc onload*[T: LoadableResource](self: T, ok: bool, err: string, p: pointer, len: int) = +proc onload*[T: LoadableResource](self: T, ok: bool, err: string, data = SliceMem[byte]()) = if self.status == Started: self.status = if ok: Finished else: Error - if self.result != nil: - self.result[] = (ok, err, p, len) + # if self.result != nil: + # self.result[] = (ok, err, data) if self.onload_func != nil: if self.use_threads: - to_return.send((self.LoadableResource, ok, err, p, len)) + to_return.send((self.LoadableResource, ok, err, data)) else: - self.onload_func(ok, err, p, len) - else: # cancelled - self.doneImpl() - -# TODO: check if we can always use destructors instead of calling this -proc done*[T: LoadableResource](self: T) = - if self.use_threads: - to_done.send self - else: - self.doneImpl() + self.onload_func(ok, err, data) proc cancel*[T: LoadableResource](self: T) = if self.status != Started: return - if self.cancel_func == nil: - # self.`onload=`(proc(ok, err, p, len: auto) = - # self.done()) - self.onload_func = proc(ok: bool, err: string, p: pointer, len: int) = - self.done() - else: + if self.cancel_func != nil: self.cancel_func() self.status = NotStarted @@ -158,24 +135,22 @@ proc workerThreadProc() {.thread.} = worker.createThread(workerThreadProc) to_start.open() to_return.open() -to_done.open() proc updateLoadableWorkerThreads*() = while true: let tried = to_return.tryRecv() if not tried.dataAvailable: break - let (res, ok, err, p, len) = tried.msg - res.onload_func(ok, err, p, len) - while true: - let tried = to_done.tryRecv() - if not tried.dataAvailable: - break - tried.msg.done_func() + let (res, ok, err, data) = tried.msg + res.onload_func(ok, err, data) + +proc terminateLoadableWorkerThreads*() = + # TODO: test this + to_start.send(nil.LoadableResource) + worker.joinThread() type Fetch* = ref object of LoadableResource custom: pointer - auto_done: bool when not defined(onlyLocalFiles): when defined(emscripten): @@ -258,16 +233,14 @@ func escapeUTF8*(s: string): string = proc loadUri*( uri: string, - onload_func: proc(ok: bool, err: string, data: pointer, len: int) = nil, + onload_func: proc(ok: bool, err: string, data: SliceMem[byte]) = nil, range = (-1,-1), auto_start = true, - auto_done = true, ): Fetch {.discardable.} = echo "fetching ", uri var start_func: proc(self: LoadableResource) - var done_func: proc() var self: Fetch var uri = uri var use_threads = false @@ -289,22 +262,16 @@ proc loadUri*( attr.onsuccess = proc(fetch: ptr emscripten_fetch_t) {.cdecl.} = var self = cast[Fetch](fetch.userData) self.custom = fetch.pointer - self.onload(true, "", fetch.data, fetch.numBytes.int) - if self.auto_done: - self.done() + self.onload(true, "", newSliceMem(fetch.data, fetch.numBytes.int, proc() = + emscripten_fetch_close(fetch) + )) attr.onerror = proc(fetch: ptr emscripten_fetch_t) {.cdecl.} = var self = cast[Fetch](fetch.userData) let err_msg = $fetch.statusText.addr.cstring let err = emscripten_fetch_close(fetch) # self.onload(false, &"Error: {err_msg} ({EMSCRIPTEN_RESULT_STRINGS[err.int + 8]})", nil, 0) - self.onload(false, &"Error fetching {fetch.url}: {err_msg}", nil, 0) - if self.auto_done: - self.done() + self.onload(false, &"Error fetching {fetch.url}: {err_msg}") discard emscripten_fetch(attr, uri.cstring) - done_func = proc() = - if self.custom != nil: - discard emscripten_fetch_close(cast[ptr emscripten_fetch_t](self.custom)) - self.custom = nil else: use_threads = true var client = newHttpClient() @@ -316,37 +283,27 @@ proc loadUri*( response = client.getContent(uri) ok = true except: - self.onload(false, &"Error fetching {uri}: {getCurrentExceptionMsg()}", nil, 0) + self.onload(false, &"Error fetching {uri}: {getCurrentExceptionMsg()}") if ok: - let p = response[0].addr - self.onload(true, "", p, response.len) - if self.auto_done: - self.done() - done_func = proc() = - response = "" + self.onload(true, "", newSliceMem(response[0].addr.pointer, response.len, proc() = + discard response + )) if not is_remote: start_func = proc(self: LoadableResource) = - when not defined(release): - var done_called = false try: var memfile = memfiles.open(uri, mode=fmRead) - self.done_func = proc() = - when not defined(release): - assert not done_called, "Done is being called multiple times. Did you forget to set auto_done = false?" - done_called = true - memfile.close() - self.onload(true, "", memfile.mem, memfile.size) - # TODO!!!! check whether these objects are freed - # and if we have to add a destructor, or what + self.onload(true, "", newSliceMem(memfile.mem, memfile.size, proc() = + try: + memfile.close() + # this should never happen but destructors require it + except OSError: discard + )) except OSError: - self.onload(false, "Could not open file: " & uri, nil, 0) - if cast[Fetch](self).auto_done: - self.done() + self.onload(false, "Could not open file: " & uri) proc str(): string = uri - self = newLoadableResource[Fetch](start_func, done_func, str, use_threads=use_threads) + self = newLoadableResource[Fetch](start_func, str, use_threads=use_threads) self.onload_func = onload_func - self.auto_done = auto_done if auto_start: start(self) return self diff --git a/src/gpu_formats/texture_decode.nim b/src/gpu_formats/texture_decode.nim index 8981150..0259ef0 100644 --- a/src/gpu_formats/texture_decode.nim +++ b/src/gpu_formats/texture_decode.nim @@ -109,31 +109,34 @@ proc getDimensionsFormat*(p: pointer, len: int): (int, int, TextureFormat) = hdr.int * toHDR + is16.int * (R_u16.int-R_u8.int)).TextureFormat return (width, height, format) -proc loadFileFromPointersLen*(tex: Texture, pointers: seq[(pointer, int)], - callback: proc(tex: Texture, p: pointer), flip = true) = +proc loadFileFromSlices*(tex: Texture, slices: seq[SliceMem[byte]], + callback: proc(tex: Texture, data: SliceMem[byte]), flip = true) = when not defined(nimdoc): assert tex.tex_type != TexCube, "Loading a cube texture from file is not supported yet" let layer_stride = tex.width * tex.height * tex.format.stride var multilayer_buffer: ArrRef[byte] - assert tex.depth == pointers.len + assert tex.depth == slices.len if tex.depth > 1: multilayer_buffer = newArrRef[byte](layer_stride * tex.depth) var pos = 0 - for (p, len) in pointers: + for slice in slices: when defined(myouUsePixie): var image: Image + proc destructor() = discard image else: var image: imagePixelData[byte] var image_16: imagePixelData[uint16] var image_f: imagePixelData[float32] + proc destructor() = + discard image; discard image_16; discard image_f var buffer: ArrRef[byte] # a reference to this pointer is kept with one of the vars above var pixels_ptr: pointer var pixels_len: int var flip = flip - if isEXR(p, len): - let (width, height, pixels) = decodeEXR(p, len) + if isEXR(slice.data, slice.byte_len): + let (width, height, pixels) = decodeEXR(slice.data, slice.byte_len) assert width == tex.width and height == tex.height, "Image size mismatch" buffer = pixels.to byte pixels_ptr = buffer[0].addr @@ -148,8 +151,8 @@ proc loadFileFromPointersLen*(tex: Texture, pointers: seq[(pointer, int)], setFlipVerticallyOnLoad(flip) flip = false var w,h,c = 0 - if isHDRFromMemory(p.toOpenArrayByte(0, len-1)): - image_f = loadFFromMemory(p.toOpenArrayByte(0, len-1), w,h,c,0) + if isHDRFromMemory(slice.toOpenArrayByte): + image_f = loadFFromMemory(slice.toOpenArrayByte, w,h,c,0) pixels_ptr = image_f.data pixels_len = image_f.byteLen when myouConvertHdrToFloat16: @@ -158,12 +161,12 @@ proc loadFileFromPointersLen*(tex: Texture, pointers: seq[(pointer, int)], cast[ptr UncheckedArray[Float16]](pixels_ptr), image_f.len) pixels_len = pixels_len div 2 - elif is16BitFromMemory(p.toOpenArrayByte(0, len-1)): - image_16 = load16FromMemory(p.toOpenArrayByte(0, len-1), w,h,c,0) + elif is16BitFromMemory(slice.toOpenArrayByte): + image_16 = load16FromMemory(slice.toOpenArrayByte, w,h,c,0) pixels_ptr = image_16.data pixels_len = image_16.byteLen else: - image = loadFromMemory(p.toOpenArrayByte(0, len-1), w,h,c,0) + image = loadFromMemory(slice.toOpenArrayByte, w,h,c,0) pixels_ptr = image.data pixels_len = image.len assert layer_stride == pixels_len, @@ -171,11 +174,11 @@ proc loadFileFromPointersLen*(tex: Texture, pointers: seq[(pointer, int)], if flip: swap_lines(pixels_ptr, tex.width * tex.format.stride, tex.height) if tex.depth == 1: - callback(tex, pixels_ptr) + callback(tex, newSliceMem(pixels_ptr, pixels_len, destructor)) return copyMem(multilayer_buffer[pos].addr, pixels_ptr, layer_stride) pos += layer_stride - callback(tex, multilayer_buffer[0].addr) + callback(tex, multilayer_buffer.toSliceMem) proc swap_lines(p: pointer, line_stride, line_count: int) = diff --git a/src/graphics/texture.nim b/src/graphics/texture.nim index 528e444..ae7914d 100644 --- a/src/graphics/texture.nim +++ b/src/graphics/texture.nim @@ -49,7 +49,8 @@ proc bind_it*(texture: Texture, reserve_slot: static[int32] = -1, needs_active_t proc unbind*(texture: Texture) proc unbindAllTextures*() proc destroy*(texture: Texture) -proc loadFromPixels*(self: Texture, pixels: pointer) +proc loadFromPixelsPointer*(self: Texture, pixels: pointer) +proc loadFromPixels*[T](self: Texture, pixels: SliceMem[T]) proc loadCubeSideFromPixels*(self: Texture, pixels: pointer, side: int32 = 0) proc setFilter*(self: Texture, filter: TextureFilter) proc newTexture*(engine: MyouEngine, name: string, width, height: int, depth: int = 1, @@ -59,7 +60,7 @@ proc newTexture*(engine: MyouEngine, name: string, width, height: int, depth: in pixels: ArrRef[float32] = nil): Texture proc generateMipmap*(self: Texture) func to_sRGB*(format: TextureFormat): TextureFormat -proc newTexture*(engine: MyouEngine, name: string, p: pointer, len: int, is_sRGB: bool, filter: TextureFilter = Trilinear, depth=1, flip=true): Texture +proc newTexture*(engine: MyouEngine, name: string, data: SliceMem[byte], is_sRGB: bool, filter: TextureFilter = Trilinear, depth=1, flip=true): Texture proc newTexture*(engine: MyouEngine, name: string, file_name: string, is_sRGB: bool, filter: TextureFilter = Trilinear, tex_type: TextureType = Tex2D, @@ -327,7 +328,7 @@ proc destroy*(texture: Texture) = texture.unbind() texture.freeTextureStorage() -proc loadFromPixels*(self: Texture, pixels: pointer) = +proc loadFromPixelsPointer*(self: Texture, pixels: pointer) = let ts = self.storage self.loaded = true self.bind_it(needs_active_texture=true) @@ -344,6 +345,9 @@ proc loadFromPixels*(self: Texture, pixels: pointer) = if self.needsMipmap: glGenerateMipmap(ts.target) +proc loadFromPixels*[T](self: Texture, pixels: SliceMem[T]) = + self.loadFromPixelsPointer(pixels.data) + proc loadCubeSideFromPixels*(self: Texture, pixels: pointer, side: int32 = 0) = let ts = self.storage self.loaded = true @@ -416,7 +420,7 @@ proc newTexture*(engine: MyouEngine, name: string, self.engine.renderer.enqueue proc()= self.ensure_storage() if pixels != nil: - self.loadFromPixels(pixels.toPointer) + self.loadFromPixelsPointer(pixels.toPointer) else: self.preallocate() when defined(myouUseRenderdoc): @@ -436,14 +440,14 @@ func to_sRGB*(format: TextureFormat): TextureFormat = of RGB_u8: SRGB_u8 else: raise newException(ValueError, "There's no sRGB version of " & $format) -proc newTexture*(engine: MyouEngine, name: string, p: pointer, len: int, is_sRGB: bool, filter: TextureFilter = Trilinear, depth=1, flip=true): Texture = +proc newTexture*(engine: MyouEngine, name: string, data: SliceMem[byte], is_sRGB: bool, filter: TextureFilter = Trilinear, depth=1, flip=true): Texture = # TODO: replace this function by one taking a LoadableResource - var (width, height, format) = getDimensionsFormat(p, len) + var (width, height, format) = getDimensionsFormat(data.data, data.byte_len) if is_sRGB: format = format.to_sRGB let self = engine.newTexture(name, width, height, depth, format, filter=filter) engine.renderer.enqueue proc() = - self.loadFileFromPointersLen(@[(p, len)], loadFromPixels, flip=flip) + self.loadFileFromSlices(@[data], loadFromPixels, flip=flip) self.loaded = true return self @@ -457,9 +461,9 @@ proc newTexture*(engine: MyouEngine, name: string, file_name: string, is_sRGB: b # note: format does not matter at this point, will be replaced later let self = engine.newTexture(name, 0, 0, filter=filter, format=RGBA_u8) var res: LoadableResource - proc load(ok: bool, err: string, p: pointer, len: int) = + proc load(ok: bool, err: string, data: SliceMem[byte]) = assert ok, &"Error loading texture '{self.name}' from '{file_name}': {err}" - let ktx_info = GetDdsKtxInfo(p, len) + let ktx_info = GetDdsKtxInfo(data.data, data.byte_len) if ktx_info.isSome: let ktx_info = ktx_info.get assert ktx_info.is_cubemap == (self.tex_type == TexCube) @@ -472,7 +476,7 @@ proc newTexture*(engine: MyouEngine, name: string, file_name: string, is_sRGB: b engine.renderer.enqueue proc()= try: self.ensure_storage() - self.loadCompressedData(ktx_info, ParseDdsKtx(p, len)) + self.loadCompressedData(ktx_info, ParseDdsKtx(data.data, data.byte_len)) self.setMipmapRange(0, ktx_info.num_mipmaps - 1) self.loaded = true # except Exception as e: @@ -482,10 +486,8 @@ proc newTexture*(engine: MyouEngine, name: string, file_name: string, is_sRGB: b # echo line echo getCurrentExceptionMsg() echo "Error loading texture " & file_name - finally: - res.done() else: - var (width, height, format) = getDimensionsFormat(p, len) + var (width, height, format) = getDimensionsFormat(data.data, data.byte_len) if is_sRGB: format = format.to_sRGB self.width = width @@ -494,15 +496,13 @@ proc newTexture*(engine: MyouEngine, name: string, file_name: string, is_sRGB: b engine.renderer.enqueue proc()= try: self.ensure_storage() - self.loadFileFromPointersLen(@[(p, len)], loadFromPixels) + self.loadFileFromSlices(@[data], loadFromPixels) self.loaded = true except: # TODO: use logging echo getCurrentExceptionMsg() echo "Error loading texture " & file_name - finally: - res.done() - res = file_name.loadUri(load, auto_start=false, auto_done=false) + res = file_name.loadUri(load, auto_start=false) res.start() return self diff --git a/src/loaders/blend.nim b/src/loaders/blend.nim index e05975b..5cd4605 100644 --- a/src/loaders/blend.nim +++ b/src/loaders/blend.nim @@ -32,6 +32,7 @@ {.warning[UseBase]: off.} # can't seem to satisy this... {.warning[UnusedImport]: off.} # for ./loader_base +{.warning[rsemMethodLockMismatch]: off.} # TODO: is this important? const myouUseBlendLoader {.booldefine.} = true @@ -90,7 +91,6 @@ proc registerBlendLoader*(engine: MyouEngine) = method close*(self: BlendLoader) = if self.resource != nil: - self.resource.done() self.resource = nil self.blend_file = nil self.cached_materials.clear() @@ -107,14 +107,12 @@ proc loadAsync(self: BlendLoader, callback: proc()) = else: self.close() self.resource = loadUri(self.blend_file_path, - proc(ok, err, data, len: auto) = + proc(ok, err, data: auto) = if ok: - self.blend_file = openBlendFile(self.blend_file_path, data, len) + self.blend_file = openBlendFile(self.blend_file_path, data.data, data.byte_len) callback() else: raise IOError.newException err - , - auto_done = false ) type BlenderObTypes = enum @@ -180,8 +178,10 @@ method loadTextureImpl*(self: BlendLoader, name: string, img: FNode): Texture = # todo: was this necessary? # let seek = img.packedfile.seek.i32[0] let size = img.packedfile.size.i32[0] - let arr = img.packedfile.data.get_array(size, char) - return self.engine.newTexture(name, arr, size, is_sRGB) + let arr = img.packedfile.data.get_array(size, byte) + # TODO: get SliceMems from blend file instead of UncheckedArray + let s = SliceMem[byte](data: arr, byte_len: size) + return self.engine.newTexture(name, s, is_sRGB) else: # echo "loading image as REGULAR file" if self.path_handler != nil: @@ -367,7 +367,7 @@ proc loadObjectImpl(self: BlendLoader, scene: Scene, obn: FNode): (GameObject, s PointLight let color = vec3(data.r.f32[0], data.g.f32[0], data.b.f32[0]) let mode = data.mode.i32[0] - let use_shadow = mode.testBit(0) + # let use_shadow = mode.testBit(0) let use_dist = mode.testBit(20) let cutoff = if use_dist: data.att_dist.f32[0] else: 0'f32 var ob = self.engine.new_light( diff --git a/src/loaders/blend_format.nim b/src/loaders/blend_format.nim index 96b730d..d7dfe20 100644 --- a/src/loaders/blend_format.nim +++ b/src/loaders/blend_format.nim @@ -121,13 +121,6 @@ proc hash*(n: FNode): Hash = # NOTE: assuming read only data (otherwise we should hash the contents) hash (n.p, n.count) -# proc `=destroy`(self: var BlendFileVal) = -# try: -# self.mem_file.close() -# except OSError: -# # this should never happen but compiler requires it for hooks -# discard - const BASIC_TYPE_LENGTHS = [1, 1, 2, 2, 4, 4, 4, 4, 8, 8, 8, 0, 1] template `[]`*(blocks: NamedBlocks, s: string): untyped = blocks[[s[0],s[1]]]