kpovmodeler not working with povray 3.7 #2
Merged
SlavekB
merged 1 commits from bug/2765/povray3.7 into master 8 months ago
MicheleC
SlavekB
Loading…
Reference in new issue
There is no content yet.
Delete Branch 'bug/2765/povray3.7'
Deleting a branch is permanent. It CANNOT be undone. Continue?
A small fix to make it work with povray3.7
https://bugs.trinitydesktop.org/show_bug.cgi?id=2765
Povray 3.7 does not allow radiosity? Or is it necessary to use other option?
In any case, this patch needs further work.
AFAIR this option is gone
http://www.povray.org/documentation/3.7.0/r3_2.html#r3_2_8_8
as I mentioned in the original bug a lot needs to be reworked, but the purpose of this patch is to make it at least rendering the model, as otherwise it renders the software mostly useless.
regards
Because it is no longer necessary to deal with compatibility with older versions of povray, so there is no need to leave the code.
// if( m_radiosity )// cl.append( TQString( "+QR" ) );// else// cl.append( TQString( "-QR" ) );Instead of leaving the code, it is now appropriate to delete the code, including related GUI options.
8bcebad8ecto23b8e490478 months agoLooks ok to me. Will wait for final approval from Slavek since he has followed this from the beginning.
It seems insufficient for me. My idea is to advance and remove the Radiosity checkbox from the GUI and the corresponding code. Now there is in the GUI checkbox that has no effect at all, which seems confusing.
There is indeed extra code related to radiosity, I also noticed this morning. The key point is to make sure we remove what is no longer required and whether to do it in this PR or as a separate one, while using this PR only to avoid setting unused (unsupported?? - I don't know) parameters anymore.
Sorry, I never knew there is option for radiosity in the GUI.
I think the respective checkbox is here: pmrendermodesdialog.cpp:350. At the same time, there will be unnecessary methods
setRadiosity(...),radiosity()and variablem_radiosity.yes, obviously there is more work to do. I will have a look, update the code and request review again.
I'll change this to WIP
It looks good, I propose one editing regarding modes 9, 10 and 11.
See comment below.
I18N_NOOP( "9: Compute media" ),I18N_NOOP( "10: Compute radiosity but no media" ),I18N_NOOP( "11: Compute radiosity and media" )I18N_NOOP( "9: Compute media" )According to documentation, modes 10 and 11 remain available, however, they seem to be identical to 9. I propose to do an item as such as 6, 7… => give 9, 10, 11… in one item and update description by documentation. The description could also be updated for item 6, 7….
See https://wiki.povray.org/content/Reference:Tracing_Options#Quality_Settings
Sounds like a good idea.
I also think c_qualityToIndex needs to be updated accordingly and I hope the last change does this correctly. So if OK, I will squash the commits.
I think this last commit is also necessary. I do not understand why it would return numQuality-1
It looks good. Everything seems to me okay.
Please check and clarify the comment below.
index = 0;if( index >= numQuality )index = numQuality - 1;if( index > numQuality )Is this change correct? Old code was setting the limit of the index to "numQuality - 1", while the new code is going up to "numQuality" and then using the value to access the array.
If we call the function indexToQuality() with argument value equals to "numQuality" we are going to do an invalid access to the element past the last in the array.
The original code seems correct to me.
Yes, you seem to be the truth –
numQualitymentions the size, so theindexmust be one smaller.1f1a0cb713to72d4668eea8 months ago@MicheleC thank you, I have overlooked this.
Now fixed this and squashed into one commit.
Looks good now.
no worries 😄
Thanks, now all seems good.
72d4668eeainto master 8 months agoReviewers
72d4668eea.