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
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
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.
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.
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 variable m_radiosity.
> Sorry, I never knew there is option for radiosity in the GUI.
I think the respective checkbox is here: [pmrendermodesdialog.cpp:350](src/branch/master/kpovmodeler/pmrendermodesdialog.cpp#L350). At the same time, there will be unnecessary methods `setRadiosity(...)`, `radiosity()` and variable `m_radiosity`.
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 variable m_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
> > Sorry, I never knew there is option for radiosity in the GUI.
>
> I think the respective checkbox is here: [pmrendermodesdialog.cpp:350](src/branch/master/kpovmodeler/pmrendermodesdialog.cpp#L350). At the same time, there will be unnecessary methods `setRadiosity(...)`, `radiosity()` and variable `m_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
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….
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
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.
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.
MicheleC
SlavekB
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.
8bcebad8ec，至23b8e49047Looks 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.1f1a0cb713，至72d4668eea@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.
72d4668eea到 master评审人
72d4668eea被合并。