kpovmodeler: +QR/-QR break povray 3.7 #35

Closed
opened 8 months ago by Dr_Nikolaus_Klepp · 17 comments

povray 3.7 does not have the +QR/-QR commandline switches any more, but kpovmodeler calls povray always wuth one of these 2 switches --> povray fails to render any image.

How to solve it: remove -QR/+QR

povray 3.7 does not have the +QR/-QR commandline switches any more, but kpovmodeler calls povray always wuth one of these 2 switches --> povray fails to render any image. How to solve it: remove -QR/+QR
SlavekB changed title from kpovmodeler: +QR/-QR brek povray 3.7 to kpovmodeler: +QR/-QR break povray 3.7 8 months ago
Owner

This is related to pull-request #2, which unfortunately needs to improve.

This is related to pull-request #2, which unfortunately needs to improve.
Collaborator

@Dr_Nikolaus_Klepp if you know exactly what has to be done, let me know. I did not have the time to read all documentation and code to see what needs to change in kpovmodeler, so that we replace this function.
If you provide analyses, it might be we fix this together.

@Dr_Nikolaus_Klepp if you know exactly what has to be done, let me know. I did not have the time to read all documentation and code to see what needs to change in kpovmodeler, so that we replace this function. If you provide analyses, it might be we fix this together.

+QR/-QR are in kpovmodeler/pmrendermode.cpp, lines 190 - 193, so for a start just comment out these lines. For better compatibility use the newer sqitches (which I don't have at hand right now):

   if( m_radiosity )
      cl.append( TQString( "+QR" ) );
   else
      cl.append( TQString( "-QR" ) );
+QR/-QR are in kpovmodeler/pmrendermode.cpp, lines 190 - 193, so for a start just comment out these lines. For better compatibility use the newer sqitches (which I don't have at hand right now): ``` if( m_radiosity ) cl.append( TQString( "+QR" ) ); else cl.append( TQString( "-QR" ) ); ```
Owner

@Dr_Nikolaus_Klepp @deloptes
Nik, Emanoil, it would be great if you can work together to come up with a fix. Then we can merge it in 😄
Please check if on older distros those options are still available. If so, we will need some conditional blocks based on version of povray.

@Dr_Nikolaus_Klepp @deloptes Nik, Emanoil, it would be great if you can work together to come up with a fix. Then we can merge it in :smile: Please check if on older distros those options are still available. If so, we will need some conditional blocks based on version of povray.
Collaborator

@MicheleC , @Dr_Nikolaus_Klepp ,
I don't mind if Nik can do the analyses in some form.
I checked back then and yes on older versions the option is there. It was removed together with other changes in the interface, so Slavek is right that it is better to update kpovmodeler to support >= 3.7
In my build process locally I just apply the patch, and it works but I agree it is not acceptible as a fix.

@MicheleC , @Dr_Nikolaus_Klepp , I don't mind if Nik can do the analyses in some form. I checked back then and yes on older versions the option is there. It was removed together with other changes in the interface, so Slavek is right that it is better to update kpovmodeler to support >= 3.7 In my build process locally I just apply the patch, and it works but I agree it is not acceptible as a fix.

Well, +-QR is the only incompatibility I came caross. Povray 3.7 is in debian since strech, so IMO the patch is all that's needed.

The compatibility with povray 3.1/3.5 can be removed, but it'll take some time for me to dig into it (have not programmed QT since Sharp Zaurus) - but I'll do it :)

Oh, most/all new features of povray can be used, when you relace the binary name in the settings dialog with a custom shell script.

Well, +-QR is the only incompatibility I came caross. Povray 3.7 is in debian since strech, so IMO the patch is all that's needed. The compatibility with povray 3.1/3.5 can be removed, but it'll take some time for me to dig into it (have not programmed QT since Sharp Zaurus) - but I'll do it :) Oh, most/all new features of povray can be used, when you relace the binary name in the settings dialog with a custom shell script.
Owner

Ok, the situation seems to be simplified because all currently supported distributions contain povray >= 3.7 – including Jessie, Trusty now does not contain povray at all.

As a result, we can move forward by deleting this option and the appropriate code without dealing with the detection of the current version of the povray.

@deloptes, please you can update pull-request #2?

Ok, the situation seems to be simplified because all currently supported distributions contain povray >= 3.7 – including Jessie, Trusty now does not contain povray at all. As a result, we can move forward by deleting this option and the appropriate code without dealing with the detection of the current version of the povray. @deloptes, please you can update pull-request #2?
Collaborator

@SlavekB and @Dr_Nikolaus_Klepp I am not sure if there was something related to the radiosity. I would be more careful with, but I will try to come back to you after double checking.

@SlavekB and @Dr_Nikolaus_Klepp I am not sure if there was something related to the radiosity. I would be more careful with, but I will try to come back to you after double checking.
Collaborator

https://www.povray.org/download/changes.txt

and search for "radiosity". My concern is that as @SlavekB objected originally there are code changes that would influence the kpovmodeler code related to readiosity and this needs a proper follow up.

IMO we could remove the option now, so that we have a quick fix for broken functionality, but keep the issue open, so that we follow up on the radiosity appropriatery - as you wish.

https://www.povray.org/download/changes.txt and search for "radiosity". My concern is that as @SlavekB objected originally there are code changes that would influence the kpovmodeler code related to readiosity and this needs a proper follow up. IMO we could remove the option now, so that we have a quick fix for broken functionality, but keep the issue open, so that we follow up on the radiosity appropriatery - as you wish.
Owner

Good to see team effort 😄

@deloptes
About radiosity, if there is a separate issue with that, I suggest we track it with a separate PR. It is better to keep things clean and avoid tracking multiple problems on the same PR if we expect one to be fix and the other to remain open for a while.

Good to see team effort :smile: @deloptes About radiosity, if there is a separate issue with that, I suggest we track it with a separate PR. It is better to keep things clean and avoid tracking multiple problems on the same PR if we expect one to be fix and the other to remain open for a while.

Radiosity is handled differently in 3.7 and up than it was in 3.5 and lower. And as said, I have not found anything but these switches that brak 3.7.

Radiosity is handled differently in 3.7 and up than it was in 3.5 and lower. And as said, I have not found anything but these switches that brak 3.7.
Owner

Radiosity is handled differently in 3.7 and up than it was in 3.5 and lower. And as said, I have not found anything but these switches that brak 3.7.

I understood the same that the appropriate option was removed without a new option in his place. Therefore, it doesn't seem that there is a need to do more than remove the appropriate code and the GUI element.

> Radiosity is handled differently in 3.7 and up than it was in 3.5 and lower. And as said, I have not found anything but these switches that brak 3.7. I understood the same that the appropriate option was removed without a new option in his place. Therefore, it doesn't seem that there is a need to do more than remove the appropriate code and the GUI element.
Collaborator

OK,
then if all agree, I updated the PR and requested the review

OK, then if all agree, I updated the PR and requested the review
Collaborator

Now looked into the documentation:
https://wiki.povray.org/content/Reference:Radiosity
https://wiki.povray.org/content/Documentation:Tutorial_Section_3.7#Radiosity

Radiosity is available but command line switches are not. From what I see there are two places to change as shown in the picture I attached

Now looked into the documentation: https://wiki.povray.org/content/Reference:Radiosity https://wiki.povray.org/content/Documentation:Tutorial_Section_3.7#Radiosity Radiosity is available but command line switches are not. From what I see there are two places to change as shown in the picture I attached
Owner

According to the documentation, quality modes 10 and 11 continue to be available:

https://wiki.povray.org/content/Reference:Tracing_Options#Quality_Settings

According to the documentation, quality modes 10 and 11 continue to be available: https://wiki.povray.org/content/Reference:Tracing_Options#Quality_Settings
Collaborator

According to the documentation, quality modes 10 and 11 continue to be available:

OK I understand. This is correct and I updated the PR as you suggested.

> According to the documentation, quality modes 10 and 11 continue to be available: OK I understand. This is correct and I updated the PR as you suggested.
Owner

Fixed by #2.

Fixed by #2.
SlavekB closed this issue 8 months ago
SlavekB added this to the R14.0.11 release milestone 8 months ago
Sign in to join this conversation.
No Milestone
No Assignees
4 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.