Closed jezek closed 2 years ago
Great work. I had actually completely forgotten about your graphics code when I talked about graphics with @CBlueGH couple of weeks ago. So this pull request is very timely. Here are some quick comments about this pull request.
Unicode uses code points up to 0x10FFFF. I think it would make sense to avoid the Unicode range so that we could in theory support both Unicode and graphics. So maybe change MAX_FONT_CHAR to 0x110000. The char32_t type is actually intended to be used for UTF-32 encoding anyway.
We probably don't want to bump the version number to 5.0.0. That should be changed to 4.8.1 for now. Next release will be either 4.8.1 or 4.9.0 probably.
@CBlueGH @mhirki Oh sorry I forgot to reply. I see you merged and I haven't changed the version, nor the MAX_FONT_CHAR constant. I'm working on the graphics for Windows client. Having some troubles with mask, but I think I'm really close to figure it out. I thought I'll do the Windows version first and than the other things @mhirki asked me to.
Is there anything I should do first, before the windows graphics?
I changed the version already. I wrote you on discord.
@CBlueGH I don't recall to have a discord account. I'm pretty sure you wrote someone else. Please disassociate that person/account with me.
Oh lol, I mixed something up, shouldn't do this at 3 am in the morning I guess xD sorry. Library-wise I migrated from SDL to SDL2 by the way, you probably saw that. Jfyi in case it is somehow relevant. Anyway, go ahead I guess. There is this client compile error though:
gcc -pipe -Wall -DUSE_X11 -DUSE_GCU -I/usr/X11R6/include -D_XOPEN_SOURCE -D_BSD_SOURCE -DMEXP=19937 -std=c99 -DSOUND_SDL sdl2-config --cflags
-D_DEFAULT_SOURCE -DACC32 -fPIE -Wno-format-truncation -fstack-protector -D_FORTIFY_SOURCE=2 -pie -fPIE -Iserver -Iserver/lua -DTEST_CLIENT -g3 -O0 -Wno-stringop-overflow -Wno-cpp -Wno-format-overflow -o client/main-x11.o -c client/main-x11.c
client/main-x11.c: In function ‘init_x11’:
client/main-x11.c:2684:33: error: case label does not reduce to an integer constant
2684 | case ReadBMPNoFile: printf("can not be read.\n"); break;
client/main-x11.c:2685:33: error: case label does not reduce to an integer constant
2685 | case ReadBMPReadErrorOrUnexpectedEOF: printf("read error or unexpected end.\n"); break;
client/main-x11.c:2686:33: error: case label does not reduce to an integer constant
2686 | case ReadBMPInvalidFile: printf("has incorrect BMP file format.\n"); break;
client/main-x11.c:2687:33: error: case label does not reduce to an integer constant
2687 | case ReadBMPNoImageData: printf("contains no image data.\n"); break;
client/main-x11.c:2688:33: error: case label does not reduce to an integer constant
2688 | case ReadBMPUnexpectedEOF: printf("unexpected end.\n"); break;
client/main-x11.c:2689:33: error: case label does not reduce to an integer constant
2689 | case ReadBMPIllegalBitCount: printf("has illegal bit count, only 24bit and 32bit images are allowed.\n"); break;
make: *** [makefile:510: client/main-x11.o] Error 1`
@CBlueGH
... There is this client compile error though:
Hmm, I didn't know, you cannot use variables in case. Weird my compiler didn't throw an error.
What I understood from the stackoverflow article above, it should go away, if you change the read bmp error constants to hardcoded constants.
#define ReadBMPNoFile -1
#define ReadBMPInvalidFile -2
#define ReadBMPNoImageData -3
#define ReadBMPUnexpectedEOF -4
#define ReadBMPReadErrorOrUnexpectedEOF -5
#define ReadBMPIllegalBitCount -6
Also the cammel case names of errors should be converted to uppercase with underscores (READ_BMP_NO_FILE) to follow hardcoded constants naming convention.
I will leave this to you, I'll continue my struggle with Windows graphics. Or should I fix this first?
I fixed the constants, go on with Windows. Maybe your compiler had some out of spec options or extensions (GNUxx/Cxx) enabled.
Hm, the patches broke some packet version compatibility. Fixed it and pushed.
@CBlueGH
Hm, the patches broke some packet version compatibility. Fixed it and pushed.
Whoa. I did miss that bug. Tried to test it with all possible client/server version combinations, but this one slipped on me. Thanks for spotting and correcting and I'm sorry for that.
Maybe your compiler had some out of spec options or extensions (GNUxx/Cxx) enabled. Hmm, I didn't tinker with any compiler settings. I'm using manjaro gnome, this is my gcc version:
$ gcc -v Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/lto-wrapper Target: x86_64-pc-linux-gnu Configured with: /build/gcc/src/gcc/configure --enable-languages=c,c++,ada,fortran,go,lto,objc,obj-c++ --enable-bootstrap --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/ --with-linker-hash-style=gnu --with-system-zlib --enable-__cxa_atexit --enable-cet=auto --enable-checking=release --enable-clocale=gnu --enable-default-pie --enable-default-ssp --enable-gnu-indirect-function --enable-gnu-unique-object --enable-linker-build-id --enable-lto --enable-multilib --enable-plugin --enable-shared --enable-threads=posix --disable-libssp --disable-libstdcxx-pch --disable-werror --with-build-config=bootstrap-lto --enable-link-serialization=1 Thread model: posix Supported LTO compression algorithms: zlib zstd gcc version 12.1.0 (GCC)
$ gcc -Q --help=target
The following options are target specific:
-m128bit-long-double [enabled]
-m16 [disabled]
-m32 [disabled]
-m3dnow [disabled]
-m3dnowa [disabled]
-m64 [enabled]
-m80387 [enabled]
-m8bit-idiv [disabled]
-m96bit-long-double [disabled]
-mabi= sysv
-mabm [disabled]
-maccumulate-outgoing-args [disabled]
-maddress-mode= long
-madx [disabled]
-maes [disabled]
-malign-data= compat
-malign-double [disabled]
-malign-functions= 0
-malign-jumps= 0
-malign-loops= 0
-malign-stringops [enabled]
-mamx-bf16 [disabled]
-mamx-int8 [disabled]
-mamx-tile [disabled]
-mandroid [disabled]
-march= x86-64
-masm= att
-mavx [disabled]
-mavx2 [disabled]
-mavx256-split-unaligned-load [disabled]
-mavx256-split-unaligned-store [disabled]
-mavx5124fmaps [disabled]
-mavx5124vnniw [disabled]
-mavx512bf16 [disabled]
-mavx512bitalg [disabled]
-mavx512bw [disabled]
-mavx512cd [disabled]
-mavx512dq [disabled]
-mavx512er [disabled]
-mavx512f [disabled]
-mavx512fp16 [disabled]
-mavx512ifma [disabled]
-mavx512pf [disabled]
-mavx512vbmi [disabled]
-mavx512vbmi2 [disabled]
-mavx512vl [disabled]
-mavx512vnni [disabled]
-mavx512vp2intersect [disabled]
-mavx512vpopcntdq [disabled]
-mavxvnni [disabled]
-mbionic [disabled]
-mbmi [disabled]
-mbmi2 [disabled]
-mbranch-cost=<0,5> 3
-mcall-ms2sysv-xlogues [disabled]
-mcet-switch [disabled]
-mcld [disabled]
-mcldemote [disabled]
-mclflushopt [disabled]
-mclwb [disabled]
-mclzero [disabled]
-mcmodel= [default]
-mcpu=
-mcrc32 [disabled]
-mcx16 [disabled]
-mdirect-extern-access [enabled]
-mdispatch-scheduler [disabled]
-mdump-tune-features [disabled]
-menqcmd [disabled]
-mf16c [disabled]
-mfancy-math-387 [enabled]
-mfentry [disabled]
-mfentry-name=
-mfentry-section=
-mfma [disabled]
-mfma4 [disabled]
-mforce-drap [disabled]
-mforce-indirect-call [disabled]
-mfp-ret-in-387 [enabled]
-mfpmath= sse
-mfsgsbase [disabled]
-mfunction-return= keep
-mfused-madd -ffp-contract=fast
-mfxsr [enabled]
-mgeneral-regs-only [disabled]
-mgfni [disabled]
-mglibc [enabled]
-mhard-float [enabled]
-mharden-sls= none
-mhle [disabled]
-mhreset [disabled]
-miamcu [disabled]
-mieee-fp [enabled]
-mincoming-stack-boundary= 0
-mindirect-branch-cs-prefix [disabled]
-mindirect-branch-register [disabled]
-mindirect-branch= keep
-minline-all-stringops [disabled]
-minline-stringops-dynamically [disabled]
-minstrument-return= none
-mintel-syntax -masm=intel
-mkl [disabled]
-mlarge-data-threshold=
-mmemset-strategy=
-mmitigate-rop [disabled]
-mmmx [enabled]
-mmovbe [disabled]
-mmovdir64b [disabled]
-mmovdiri [disabled]
-mmove-max= 128
-mmpx [disabled]
-mms-bitfields [disabled]
-mmusl [disabled]
-mmwait [disabled]
-mmwaitx [disabled]
-mneeded [disabled]
-mno-align-stringops [disabled]
-mno-default [disabled]
-mno-fancy-math-387 [disabled]
-mno-push-args [disabled]
-mno-red-zone [disabled]
-mno-sse4 [enabled]
-mnop-mcount [disabled]
-momit-leaf-frame-pointer [disabled]
-mpc32 [disabled]
-mpc64 [disabled]
-mpc80 [disabled]
-mpclmul [disabled]
-mpcommit [disabled]
-mpconfig [disabled]
-mpku [disabled]
-mpopcnt [disabled]
-mprefer-avx128 -mprefer-vector-width=128
-mprefer-vector-width= none
-mpreferred-stack-boundary= 0
-mprefetchwt1 [disabled]
-mprfchw [disabled]
-mptwrite [disabled]
-mpush-args [enabled]
-mrdpid [disabled]
-mrdrnd [disabled]
-mrdseed [disabled]
-mrecip [disabled]
-mrecip=
-mrecord-mcount [disabled]
-mrecord-return [disabled]
-mred-zone [enabled]
-mregparm= 6
-mrelax-cmpxchg-loop [disabled]
-mrtd [disabled]
-mrtm [disabled]
-msahf [disabled]
-mserialize [disabled]
-msgx [disabled]
-msha [disabled]
-mshstk [disabled]
-mskip-rax-setup [disabled]
-msoft-float [disabled]
-msse [enabled]
-msse2 [enabled]
-msse2avx [disabled]
-msse3 [disabled]
-msse4 [disabled]
-msse4.1 [disabled]
-msse4.2 [disabled]
-msse4a [disabled]
-msse5 -mavx
-msseregparm [disabled]
-mssse3 [disabled]
-mstack-arg-probe [disabled]
-mstack-protector-guard-offset=
-mstack-protector-guard-reg=
-mstack-protector-guard-symbol=
-mstack-protector-guard= tls
-mstackrealign [disabled]
-mstore-max= 128
-mstringop-strategy= [default]
-mstv [enabled]
-mtbm [disabled]
-mtls-dialect= gnu
-mtls-direct-seg-refs [enabled]
-mtsxldtrk [disabled]
-mtune-ctrl=
-mtune= generic
-muclibc [disabled]
-muintr [disabled]
-mvaes [disabled]
-mveclibabi= [default]
-mvect8-ret-in-mem [disabled]
-mvpclmulqdq [disabled]
-mvzeroupper [enabled]
-mwaitpkg [disabled]
-mwbnoinvd [disabled]
-mwidekl [disabled]
-mx32 [disabled]
-mxop [disabled]
-mxsave [disabled]
-mxsavec [disabled]
-mxsaveopt [disabled]
-mxsaves [disabled]
Known assembler dialects (for use with the -masm= option): att intel
Known ABIs (for use with the -mabi= option): ms sysv
Known code models (for use with the -mcmodel= option): 32 kernel large medium small
Valid arguments to -mfpmath=: 387 387+sse 387,sse both sse sse+387 sse,387
Known choices for mitigation against straight line speculation with -mharden-sls=: all indirect-jmp none return
Known indirect branch choices (for use with the -mindirect-branch=/-mfunction-return= options): keep thunk thunk-extern thunk-inline
Known choices for return instrumentation with -minstrument-return=: call none nop5
Known data alignment choices (for use with the -malign-data= option): abi cacheline compat
Known vectorization library ABIs (for use with the -mveclibabi= option): acml svml
Known address mode (for use with the -maddress-mode= option): long short
Known preferred register vector length (to use with the -mprefer-vector-width= option): 128 256 512 none
Known stack protector guard (for use with the -mstack-protector-guard= option): global tls
Valid arguments to -mstringop-strategy=: byte_loop libcall loop rep_4byte rep_8byte rep_byte unrolled_loop vector_loop
Known TLS dialects (for use with the -mtls-dialect= option): gnu gnu2
Known valid arguments for -march= option: i386 i486 i586 pentium lakemont pentium-mmx winchip-c6 winchip2 c3 samuel-2 c3-2 nehemiah c7 esther i686 pentiumpro pentium2 pentium3 pentium3m pentium-m pentium4 pentium4m prescott nocona core2 nehalem corei7 westmere sandybridge corei7-avx ivybridge core-avx-i haswell core-avx2 broadwell skylake skylake-avx512 cannonlake icelake-client rocketlake icelake-server cascadelake tigerlake cooperlake sapphirerapids alderlake bonnell atom silvermont slm goldmont goldmont-plus tremont knl knm intel geode k6 k6-2 k6-3 athlon athlon-tbird athlon-4 athlon-xp athlon-mp x86-64 x86-64-v2 x86-64-v3 x86-64-v4 eden-x2 nano nano-1000 nano-2000 nano-3000 nano-x2 eden-x4 nano-x4 k8 k8-sse3 opteron opteron-sse3 athlon64 athlon64-sse3 athlon-fx amdfam10 barcelona bdver1 bdver2 bdver3 bdver4 znver1 znver2 znver3 btver1 btver2 generic native
Known valid arguments for -mtune= option: generic i386 i486 pentium lakemont pentiumpro pentium4 nocona core2 nehalem sandybridge haswell bonnell silvermont goldmont goldmont-plus tremont knl knm skylake skylake-avx512 cannonlake icelake-client icelake-server cascadelake tigerlake cooperlake sapphirerapids alderlake rocketlake intel geode k6 athlon k8 amdfam10 bdver1 bdver2 bdver3 bdver4 btver1 btver2 znver1 znver2 znver3
Hello @CBlueGH ,
I see you started to work on graphics some commits ago. A while ago, I made some changes to be able to use graphic tiles, but only for linux client. Maybe my findings and this PR can be helpful for you.
First I must warn you of the ReadBMP function, it has some old code and can read only bitmaps with specific color encodings. This I fixed by replacing (and modifying) the function with a function from some other angband project.
Also if I recall correctly the resize function contains a bug, there is equal sign missing is one condition.
Maybe this saves you some hours/days of debugging.
The biggest hurdle is, if you want to use more than 256 graphical tiles, the visual character variable has to be a bigger type. Instead of
char
, I usedchar32_t
. With this comes a lot of changes not only to client, but server too. I grouped those into the first commit of this PR.And at least, if you just use loaded graphical tiles, the game becomes paradoxically less colorful, because with font tiles you can change foreground color, which is not possible if just drawing loaded graphics tiles. I've approached this by using one color in the bitmap as mask and that mask will be colored with the foreground color.
Note: All commits have detailed description of what is done in it.
Note2: There is a companion post in forum discussing this PR and I've drawn some background tiles too.
Note3: I have some free time now, so I can try to make work the windows version.
But first I must find out how to run tomenet using wine and how to compile windows client. If I'm stuck I'll try to ask for some help here.I managed to crossbuild, wrote some instructions to forum.Note4: I bumped the version to 5.0 in this PR for testing purposes. If you decide to merge but are not satisfied with v5.0, just say, I can change it.