Jeffrey Liu (Feb 08 2020 at 16:49): Jeffrey Liu (Feb 08 2020 at 16:49):Regarding the p command, I've been trying to create a VOL primitive but this command doesn't seem to be working for me: cv hu16 nuc scandata16 scandata8, scandata8 ends up with no data. Do you know why this is happening? I'm almost positive I followed the wiki steps exactly.
Sean (Feb 08 2020 at 18:11):Nothing comes to mind @Jeffrey Liu
Sean (Feb 08 2020 at 18:11):does cv have a debug flag? if it does, that might help figure out what its doing
Sean (Feb 08 2020 at 18:13):I just tried it on a data file and it definitely downsampled 16bit data to 8bit for me
Sean (Feb 08 2020 at 18:13):what's scandata16 have in it?
Jeffrey Liu (Feb 08 2020 at 18:59):I was just following the wiki, and scandata16 had a couple megabytes.
Jeffrey Liu (Feb 08 2020 at 18:59):For now, would it be possible for you to send me the scandata8 file? I'd like to use it to test the p command on VOL.
Sean (Feb 08 2020 at 19:00):does it give you nothing if you try other types too?
Sean (Feb 08 2020 at 19:00):like huc to nuc or something similar that is a pass-through
Jeffrey Liu (Feb 08 2020 at 19:01):I can't test it at the moment but if I recall correctly, it was fine when I converted a bw to a dsp
Sean (Feb 08 2020 at 19:02):try to isolate what exactly isn't working, if other cases do work
Sean (Feb 08 2020 at 19:02):e.g., does hu16 to nu16 work or nu16 to hu16 or nuc to huc or huc to nuc etc
Sean (Feb 08 2020 at 19:03):those labels are basically just telling cv how to handle the bytes in the file
Sean (Feb 08 2020 at 19:04):so for hu16 to nu16, it convert pairs of bytes like ABABABAB... to BABABABA...
Sean (Feb 08 2020 at 19:05):hu16 to nuc should be downsampling so ABABABAB... becomes CCCC...
Sean (Feb 08 2020 at 19:06):it's curious that the tutorial is suggesting network order on the unsigned chars.. I'd suggest trying hu16 to huc, see if that results in data
Sean (Feb 08 2020 at 19:07):complete guess, but what could be happening is there may be some bug on Windows (I assume you're doing this on Windows) related to network-order and host-order bytes.
Jeffrey Liu (Feb 10 2020 at 01:50):Hm, using different combinations of '16' and 'c', only "huc" to "nu16" results in any data. It probably is a Windows thing though...
Jeffrey Liu (Feb 10 2020 at 04:21):Ok so the conversion worked in Linux, and then I tried the "p" command that applied to VOL. Out of the three commands included (ECMD_VOL_THRESH_LO, ECMD_VOL_THRESH_HI, and ECM_VOL_CSIZE), the two threshold changing commands work, but changing the voxel size doesn't (but I think it would if I removed the es_edflag == ECMD_VOL_CSIZE conditional from the SEDIT_SCALE)
Sean (Feb 10 2020 at 04:22):hm, so sounds like cv is outright broken on Windows
Sean (Feb 10 2020 at 04:23):cv is a relatively simple program. almost certainly something basic is failing like endianneess detection or file I/O in text vs binary or something similar
Jeffrey Liu (Feb 10 2020 at 04:37):So I tested the 'p' fix again by removing that one conditional from SEDIT_SCALE and it worked, but it doesn't seem like the best idea since I have no idea where else the macro is used for... That is unfortunate though, because the rest of the things all work since they only require a single argument while this one requires three.
Jeffrey Liu (Feb 10 2020 at 04:46):^ What I mean by that is my change involved checking EDIT_SCALE instead of just PSCALE and SSCALE, and it works for the most part (the "three arguments needed" no longer appears when trying to scale EBM, DSP, or VOL), but that one voxel size conditional isn't working
Jeffrey Liu (Feb 10 2020 at 04:53):If that doesn't work, then using the EDIT_SCALE check won't work. The only other alternative I can think of would be to sImply include the necessary checks in the conditional by adding more ORs. I don't see anything wrong with that, except for the fact that it's a lot messier than using a single macro.
Sean (Feb 10 2020 at 16:46):ah, hm
Sean (Feb 10 2020 at 16:55):yeah, I think the problem is that we really need macros categorized by the number of parameters instead of by type of operation
Sean (Feb 10 2020 at 16:55):since they ultimately seem to be describing how the 'p' command is to validate/behave
Sean (Feb 10 2020 at 16:56):@Jeffrey Liu would it make sense to change/replace them like that (at least the higher-level macros like EDIT SCALE)
Jeffrey Liu (Feb 11 2020 at 02:14):Actually, I had the same idea in mind and was planning on asking - maybe not change or replace the macro entirely because other code also makes use of it (unless that's fine), but add new #defines for something like ONE_ARG, TWO_ARG, etc?
Sean (Feb 11 2020 at 02:59):well you'd have to briefly look at what the other code is doing with those macros to decide whether to keep them or add to them.
Sean (Feb 11 2020 at 03:00):I wouldn't just add to them without looking as adding more symbols will be an increase in overall complexity, especially if they only get used in one place. that'd be motivation to just add more ORs using the existing symbols.
Jeffrey Liu (Feb 11 2020 at 03:09):Ok, that makes sense - I'll take a closer look at where they're being used when I get the chance. I searched it once through the entire solution, and if I recall correctly, they were used maybe around 3-4 times (that estimate may be low).
Sean (Feb 11 2020 at 03:10):+1
Sean (Feb 11 2020 at 03:10):any symbols that only get used once, unless they're part of a set, are typically good candidates for removal (to reduce complexity)
Jeffrey Liu (Feb 11 2020 at 03:11):Hm, in that case, if that's the only place where some sort of parameter count categorization is needed, then maybe it would just be better to add more ORs?
Sean (Feb 11 2020 at 03:12):unless you make a set like ONE_ARG, TWO_ARG, etc that simplify understanding ;)
Jeffrey Liu (Feb 11 2020 at 03:14):So, if I understand correctly, new macros could be defined if simply editing EDIT_SCALE is not an option?
Sean (Feb 11 2020 at 03:15):sure
Sean (Feb 11 2020 at 03:15):there's not hard rules
Sean (Feb 11 2020 at 03:16):just a desire to keep the code / API as simple as possible for the next person reading the code, but as complex as it needs to be today is fine too
Jeffrey Liu (Feb 11 2020 at 03:26):It looks like all the other occurences of SEDIT_SCALE appear in cases like this:
if (!SEDIT_SCALE)
es_edflag = SSCALE;
that are all in a switch case for am_mode. Not really sure exactly what the switch case means/does, but the usage seems important to say the least?
Jeffrey Liu (Feb 11 2020 at 03:32):Oh sorry, it's used much more than that - I was only looking in a single file. I think that with this many occurences, editing that macro may have some undesired side effects.
Jeffrey Liu (Feb 11 2020 at 03:41):Yeah, turns out it's being used 9 times in chgview.c, doevent.c, edsol.c, mged.c, and usepen.c. I haven't looked deeply into each use case, but I definitely think that it might be simpler to create a set of ONE_ARG, TWO_ARG, etc.
Jeffrey Liu (Feb 11 2020 at 03:45):Also, in order to categorize macros by parameter count, even if SEDIT_SCALE was modified to accomodate for single parameter commands, wouldn't new defines be needed for TWO_ARG and THREE_ARG? And in that case, I feel like it would be simpler to just leave SEDIT_SCALE as be and then create a separate ONE_ARG macro.
Sean (Feb 11 2020 at 03:45):sounds good, analysis performed ;)
Jeffrey Liu (Feb 11 2020 at 03:47):Cool, so I'll add the set in sedit.h. Thanks so much for your advice! I was a little stuck there, macros still are pretty confusing to me.
Sean (Feb 11 2020 at 03:47):yeah, I think you're probably right. new macros sound like they're in order.
Sean (Feb 11 2020 at 03:49):you will want to give them a proper prefix so they're consistent with other defines. for example, if they're only used in sedit.c, then something like SEDIT_1ARG, SEDIT_2ARG, etc or SEDIT_1PARAM or PARAM_1ARG, etc may make sense.
Jeffrey Liu (Feb 11 2020 at 03:53):Oh wait, so if they're being used in edsol.c (where the check is happening), should I just define it there instead of in sedit.h? I guess that makes more sense now that I think about it...
Jeffrey Liu (Feb 13 2020 at 02:39):Alright, here's my changes with the macros. I basically just transferred what was needed from the conditionals, so let me know if it's missing anything: p_command.patch
Jeffrey Liu (Feb 13 2020 at 02:39):I tested it with the EBM, DSP, and VOL scaling and it's worked for everything that I've tried so far.
Sean (Feb 14 2020 at 17:44):@Jeffrey Liu That looks like a perfect patch! Very cool, well done.
Sean (Feb 14 2020 at 17:45):That being your second flawless change also means something special too... you now have been promoted with direct commit privileges.
Sean (Feb 14 2020 at 17:47):Please read in HACKING what that means in terms of expectations, but it also means you can apply this latest patch yourself. That's also a good test just to make sure your checkout and account is set up correctly. You shouldn't need to do anything special other than "svn commit path/to/file" or run svn diff to make sure that's the only changes you've made and "svn commit" .
Sean (Feb 14 2020 at 17:53):Thank you for all your work on this!
Jeffrey Liu (Feb 14 2020 at 19:05):Wow, that's awesome!! Thanks so much - I will definitely use this privilege as wisely as I can :)
Sean (Feb 14 2020 at 20:53):s/wisely/frequently/ :-D
Sean (Feb 14 2020 at 20:55):@Jeffrey Liu You definitely earned it. Thank you for all your efforts improving ebm, dsp, and vol
Sean (Feb 16 2020 at 05:09):@Jeffrey Liu how's it going? problems getting the commit to post?
Jeffrey Liu (Feb 18 2020 at 00:49):Sorry, I was a bit busy over the weekend but I just read over HACKING again and then committed my changes. I think it worked, but let me know if I did anything wrong.
Jeffrey Liu (Feb 18 2020 at 00:49):HACKING was quite long though, I think I'll need to refer to it several more times...
starseeker (Feb 18 2020 at 01:24):That's normal - we refer to it ourselves
Jeffrey Liu (Feb 18 2020 at 02:49):Yikes sorry about the bracket thing... I had some other unrelated changes to edsol.c, so I cloned the repository again elsewhere and tried to transfer the relevant changes over to the new untouched version before committing. I guess I missed something though - would there have been a better way to do it?
starseeker (Feb 19 2020 at 00:56):The best way to catch those is to do a build test before committing
Jeffrey Liu (Feb 19 2020 at 13:25):Right - my fault, don't know why VS didn't catch something like that though
Sean (Feb 22 2020 at 17:03):Jeffrey Liu said:
Sorry, I was a bit busy over the weekend but I just read over HACKING again and then committed my changes. I think it worked, but let me know if I did anything wrong.
@Jeffrey Liu looked good to me other than you probably skipped compiling before committing given that one curly brace was missing, but otherwise it still looks like you're set up correctly and ready to commit with rest of us as you like
Sean (Feb 22 2020 at 17:04):if you have any other patches pending, feel free to get them merged in and committed too. don't be like me and have 20 branches with uncommitted work that goes stale ;)
Jeffrey Liu (Feb 22 2020 at 17:06):Cool! I think all my other work is still a WIP (Appleseed and the GUI), so I don't think it's ready to be committed.
Sean (Feb 22 2020 at 17:07):it's okay to commit works in progress as long as you protect them AND document them
Sean (Feb 22 2020 at 17:08):for example, you can wrap them in #ifdef WORK_IN_PROGRESS ... if it's changes to existing published+documented code
Sean (Feb 22 2020 at 17:08):in the case of 'art, though, that's completely new development and hasn't been announced, so there is no problem committing anything to art.cpp for example so long as you don't break compilation for others
Sean (Feb 22 2020 at 17:10):whenever something is in-progress/incomplete, it's also good to leave a comment as to that status, though we try to keep that to a minimum and commit the smallest working steps that won't affect others negatively
Sean (Feb 22 2020 at 17:10):can't commit too frequently ;)
Jeffrey Liu (Feb 22 2020 at 17:12):Ah ok, that makes sense. I'll try to commit them when I get the time. Sorry about the Appleseed light testing and stuff, I've been pretty busy with school recently.
Sean (Feb 22 2020 at 17:12):it's okay, school's important :)
Sean (Feb 27 2020 at 16:09):Jeffrey Liu said:
HACKING was quite long though, I think I'll need to refer to it several more times...
Any feedback on HACKING? Did any parts of it seem unnecessary or irrelevant or a bore or annoying or fantastic or just right or all wrong or ... Any suggestions for improving it?
Thusal Ranawaka (Aug 01 2020 at 17:33):@Sean My Ultimate VOL Masterpiece, putting my skills to an ultimate test,
Original Picture, image.png
Wireframe, image.png
Ray Traced, image.png
Thusal Ranawaka (Aug 01 2020 at 17:33):Took me 24 layers.
Thusal Ranawaka (Aug 01 2020 at 17:34):
Thusal Ranawaka (Aug 01 2020 at 17:35):I tried to assign colors also but didn't work out.
Thusal Ranawaka (Aug 01 2020 at 17:35)::grinning_face_with_smiling_eyes:
Sean (Aug 02 2020 at 02:44):looks like the original picture's base is 17x17, but yours is 11x11 ... :)
Sean (Aug 02 2020 at 02:44):otherwise, nicely done!
Thusal Ranawaka (Aug 02 2020 at 02:45):Yeah, I set it to 19x11. That's why.
Thusal Ranawaka (Aug 02 2020 at 02:46):I could make the base bigger, but I just went for the original picture's pattern. That's why, the base was small.
Sean (Aug 02 2020 at 02:46):I don't understand.
Thusal Ranawaka (Aug 02 2020 at 02:48):The base was so small, because I didn't thought about the size of the base, I thought about the trophy's hands, So I set the maximum dimensions as 19x11. That's why I have to made that base small.
Thusal Ranawaka (Aug 02 2020 at 02:49):But, I could make it bigger.
Last updated: Oct 09 2024 at 00:44 UTC